-
Notifications
You must be signed in to change notification settings - Fork 107
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 spoc merge
Command
#2136
Add spoc merge
Command
#2136
Conversation
Hi @mhils. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
} | ||
|
||
// yaml.Unmarshal happily takes YAML for a SELinux profile and unmarshals | ||
// it into SeccompProfile. We need to check the YAML kind! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This here was quite surprising to me and annoying to track down.
I think this is also a bug in Artifact.Pull(...)
, but I haven't verified it end-to-end. If we all are happy with directly using yalm.Unmarshal
there (instead of going through a.YamlUnmarshal
), I'm happy to follow up with another PR that extracts profile loading into a generic utility function which is used in both places.
In any case, that's definitely a separate PR. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We only used if for seccomp right now, so yeah. This may be a bad bug.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
} | ||
|
||
// yaml.Unmarshal happily takes YAML for a SELinux profile and unmarshals | ||
// it into SeccompProfile. We need to check the YAML kind! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We only used if for seccomp right now, so yeah. This may be a bad bug.
/ok-to-test |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2136 +/- ##
==========================================
- Coverage 45.50% 40.95% -4.55%
==========================================
Files 79 102 +23
Lines 7782 15106 +7324
==========================================
+ Hits 3541 6187 +2646
- Misses 4099 8482 +4383
- Partials 142 437 +295 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mhils, saschagrunert The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Thank you for the prompt review! 🍰 Tests are just flaky here, right? |
Yes, restarted the OLM test. |
Thanks - looks like we are extra lucky today or we have CI breakage. 🙃 |
Thank you for the super quick review! 🍰 ⚡ |
Thank you for working on this @mhils! 🙏 |
What type of PR is this?
/kind feature
What this PR does / why we need it:
This PR adds a
spoc merge
command which can be used to merge multiple security profiles via the CLI.Does this PR have test?
Yes.
Does this PR introduce a user-facing change?