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

Add script for auto-releasing bundle to FBC #766

Merged
merged 1 commit into from
Jan 15, 2025
Merged

Add script for auto-releasing bundle to FBC #766

merged 1 commit into from
Jan 15, 2025

Conversation

Allda
Copy link
Contributor

@Allda Allda commented Jan 9, 2025

The new script is introduced that automatically release a new bundle to a fbc template and renders it to catalog using opm command.

The script creates templates if not exists yet or amend template and add new bundle. The script uses a new catalog mapping in ci.yaml and new release-config.yaml file that contains info about how to release a bundle.

JIRA: ISV-5506

Merge Request Checklists

  • Development is done in feature branches
  • Code changes are submitted as pull request into a primary branch [Provide reason for non-primary branch submissions]
  • Code changes are covered with unit and integration tests.
  • Code passes all automated code tests:
    • Linting
    • Code formatter - Black
    • Security scanners
    • Unit tests
    • Integration tests
  • Code is reviewed by at least 1 team member
  • Pull request is tagged with "risk/good-to-go" label for minor changes

@Allda Allda requested review from johnbe11 and haripate January 10, 2025 08:25
Copy link
Contributor

@haripate haripate left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@johnbe11 johnbe11 left a comment

Choose a reason for hiding this comment

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

@Allda please see the two comments above - neither are necessarily blocking concerns. Otherwise, LGTM


self._template: dict[str, Any] = {}
self.template_path = (
self.operator.root / "catalog-templates" / self.template_name
Copy link
Contributor

Choose a reason for hiding this comment

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

Should self.operator be safety checked here prior to referencing self.operator.root, since operator can potentially initialize as None?
https://github.com/mporrato/operator-repo/blob/v0.4.11/src/operator_repo/core.py#L37

update: the operator appears to already be checked for None here, this appears sufficient to address this concern (please confirm) --
https://github.com/mporrato/operator-repo/blob/v0.4.11/src/operator_repo/core.py#L146

Copy link
Contributor

Choose a reason for hiding this comment

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

(note that after this instantiation, there are also a number of additional spots below where self.operator.[operator-attribute] could similarly cause uncaught exceptions if operator is None)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You don't have to worry about the attributes. The Operator-repo library is written in a way that doesn't allow you to initiate an object (Repo, Operator,...) that would not be validated first. So if we pass an invalid input to the CLI it won't even execute an initiate these objects.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ack, thanks for the confirm/clarification

self.template_type,
"-o",
"yaml",
self.template_path,
Copy link
Contributor

Choose a reason for hiding this comment

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

Are the constructor params which are originally used to create template_path checked/validated in some way before this? (wondering if command could potentially be vulnerable to command injection)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The self.operator that is used to generate the template_path is defined in our library and it always make sure the path is valid path pointing to an operator (validation is done using probes - if no valid operator the whole app fails on purpose). There are also a couple of preventative action that disallow any injection.

  • the self.template_path is a type of Path that should point to a valid path on a system
  • we run a command using a subprocess with a disabled shell
  • the command is executed using separate args given by the list variable ["opm", "alpha", "render", ....] which doesn't allow injecting any vulnerable code.

With these reasons and fact that we will call this CLI tool in our pipelines and we can control inputs I think we have enough validation around.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 15, 2025
The new script is introduced that automatically release a new bundle to
a fbc template and renders it to catalog using opm command.

The script creates templates if not exists yet or amend template and add
new bundle. The script uses a new catalog mapping in ci.yaml and new
release-config.yaml file that contains info about how to release a
bundle.

JIRA: ISV-5506

Signed-off-by: Ales Raszka <[email protected]>
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 15, 2025
@Allda Allda merged commit ec74189 into main Jan 15, 2025
3 checks passed
@Allda Allda deleted the ISV-5506 branch January 15, 2025 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants