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

Extract and expose evaluation/assertion logic for checks #642

Closed
komish opened this issue May 18, 2022 · 2 comments
Closed

Extract and expose evaluation/assertion logic for checks #642

komish opened this issue May 18, 2022 · 2 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.

Comments

@komish
Copy link
Contributor

komish commented May 18, 2022

Problems

  • In several places, we bind tasks that are important for our execution to tasks that perform data verification, comparison etc.

  • Another example might be test cases, where we need to write information to a file (say, as an artifact), and we first must verify certain elements of the data we need to validate before we write it.

  • In some cases, we also store this logic behind our internal packages, in our checks themselves.

  • There’s value, at the very least from a testing perspective, and beyond that - to users who may want to perform preflight checks within their own test code, to decoupling the logic behind validating a check’s criteria against raw data, and performing other logic.

The solution should:

  • Expose functionality that accepts data structures and evaluates our check conditions as library functions.
  • Decouple our runtime logic from this validation

One of the simplest examples (and therefore one of the locations where we’d get the least amount of value, arguably) would be the HasRequiredLabels check.

https://github.com/redhat-openshift-ecosystem/openshift-preflight/blob/main/certification/internal/policy/container/has_required_labels.go#L31-L48

This validation could effectively be a public function that we then call in this method.

Similar/Related: #639

@komish komish added the kind/feature Categorizes issue or PR as related to a new feature. label May 18, 2022
@acornett21
Copy link
Contributor

When #639 was raised I was thinking maybe we need an api or pkg package that calls our Validate function of each check. That way our validation code, need not be exposed fully and we can still rely on an internal package. There is probably more work for this issue 642, but maybe having a public layer like I mentioned would be also helpful.

@komish komish added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 4, 2022
@komish
Copy link
Contributor Author

komish commented Oct 4, 2022

Marking this issue as stale, and also in favor of #639, which is opened by the community and will be a more collaborative discussion.

@komish komish closed this as not planned Won't fix, can't repro, duplicate, stale Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
None yet
Development

No branches or pull requests

2 participants