Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce templates for new packages #14144

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

hellcp-work
Copy link
Contributor

@hellcp-work hellcp-work commented Apr 12, 2023

Introduces a simple template support to new project creation. Currently only supports one type of a package, SUSE style RPM, but it can be easily expanded to support everything that may be needed.

To verify:

  1. Create a new project for templates
  2. Set the OBS:PackageTemplates attribute on the project
  3. Create a package in the template project
  4. Create a file with name like: {{ package.name }}.extension.liquid and contents like {{ user.name }} (other options are in src/api/app/services/package_service/templater/{project,package,user}_drop.rb files)
  5. Go to any other project
  6. Click a create new package button
  7. Write metadata about the new package
  8. Select the template you created in the template project
  9. Observe the package be initialized with a set of files

TODO:

  • Create test data
  • Create tests
  • Write documentation for the feature (being worked on in a follow up PR)

@github-actions github-actions bot added the Frontend Things related to the OBS RoR app label Apr 12, 2023
@hellcp-work
Copy link
Contributor Author

@eduardoj eduardoj added the review-app Apply this label if you want a review app started label Apr 12, 2023
@obs-bot
Copy link
Collaborator

obs-bot commented Apr 12, 2023

Review app will appear here: http://obs-reviewlab.opensuse.org/hellcp-work-templates

@hennevogel
Copy link
Member

This would require code changes and deployments for every template change. I'm not sure this is such a good idea.

@hellcp-work hellcp-work marked this pull request as draft April 17, 2023 08:02
@hennevogel hennevogel removed the review-app Apply this label if you want a review app started label Oct 24, 2023
@hellcp-work hellcp-work force-pushed the templates branch 2 times, most recently from 61b1d91 to fe22bfd Compare June 6, 2024 12:07
@hellcp-work
Copy link
Contributor Author

This would require code changes and deployments for every template change. I'm not sure this is such a good idea.

I went for a similar mechanism to image templates now. I implemented basic templating with liquid, in order to prefill some metadata, to avoid having to do that manually

@hennevogel hennevogel added the review-app Apply this label if you want a review app started label Sep 30, 2024
@obs-bot
Copy link
Collaborator

obs-bot commented Sep 30, 2024

Review app will appear here: http://obs-reviewlab.opensuse.org/hellcp-work-templates

def set_template
return unless params['template']

template_project, template_package = params['template'].split('/')
Copy link
Contributor

@danidoni danidoni Sep 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if params['template'] does not have a '/' inside?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wouldn't work, I should do some validation here

@@ -105,6 +105,7 @@ def staging_history
scope :related_to_user, ->(user_id) { joins(:relationships).where(relationships: { user_id: user_id }) }
scope :for_group, ->(group_id) { joins(:relationships).where(relationships: { group_id: group_id, role_id: Role.hashed['maintainer'] }) }
scope :related_to_group, ->(group_id) { joins(:relationships).where(relationships: { group_id: group_id }) }
scope :with_package_templates, -> { joins(:attribs).where(attribs: { attrib_type_id: AttribType.joins(:attrib_namespace).where(attrib_namespace: { name: 'OBS' }, attrib_types: { name: 'PackageTemplates' }) }) }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can do the same without subqueries by adding :attrib_types into the join, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure, let me check

@hennevogel
Copy link
Member

To verify:

  • Create a new project for templates
    ...

You should do that in lib/tasks/dev/package-templates.rake and dev:test_data

@hellcp-work
Copy link
Contributor Author

You should do that in lib/tasks/dev/package-templates.rake and dev:test_data

It's done in lib/tasks/dev/templates.rake and dev:test_data

@hennevogel
Copy link
Member

I don't get it, is replacing Package/Project/User data the "feature" of those templates? Do we need to code and commit Liquid::Drop for each new object we want to fetch data from? What about data not from some object but from the form?

@hellcp-work
Copy link
Contributor Author

I don't get it, is replacing Package/Project/User data the "feature" of those templates? Do we need to code and commit Liquid::Drop for each new object we want to fetch data from? What about data not from some object but from the form?

Yes, all data accessible from the template is specified by those drops. I wanted to avoid presenting data that we don't want to present, like User#password. The things in a form will be available via PackageDrop, since that's the package the user created with that metadata. Templates are unfortunately code, and keeping a minimal amount of access to the database is a good idea, since this feature if not done carefully, is an easy attack vector.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Frontend Things related to the OBS RoR app review-app Apply this label if you want a review app started Test Suite / CI 💉 Things related to our tests/CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants