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 support to Ignition remediation type. #5137

Merged

Conversation

ggbecker
Copy link
Member

@ggbecker ggbecker commented Jan 28, 2020

Description:

  • Add support to Ignition remediation type

How to generate the remediation meanwhile OpenSCAP doesn't support this specific type:

oscap xccdf generate fix --profile coreos-ncp --template urn:xccdf:fix:script:ignition build/ssg-ocp4-ds.xml

Copy link
Member

@yuumasato yuumasato left a comment

Choose a reason for hiding this comment

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

Awesome, works like a charm.

@@ -77,6 +77,7 @@ selections:
#- sshd_set_keepalive
#- sshd_enable_warning_banner
#- sshd_rekey_limit

Copy link
Member

Choose a reason for hiding this comment

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

nitpick: Unchaning change. But I'd rather not wait for Jenkins to test it again.

@yuumasato yuumasato added this to the 0.1.49 milestone Jan 28, 2020
@isimluk
Copy link
Member

isimluk commented Jan 28, 2020

Nice! LGTM. 👍

@@ -0,0 +1,18 @@
# platform = multi_platform_all
apiVersion: machineconfiguration.openshift.io/v1
kind: Ignition
Copy link
Collaborator

Choose a reason for hiding this comment

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

The Kind should be "MachineConfig", not "Ignition".
Sorry, I know this is confusing. Ignition is the yaml-based language that is used under spec.config. But together with the other things, the spec makes up a "MachineConfig" Kubernetes type.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Great work overall by the way. I'm testing the result with the operator now.

Copy link
Member Author

Choose a reason for hiding this comment

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

It was actually a find-replace error. I will fix that.

Copy link
Member

Choose a reason for hiding this comment

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

@jhrozek while OCP may aggregate these snippets into MachineConfig's, would it be reasonable to expect use cases where other Kubernetes distributions may take the Ignition scripts for other purposes?

Or in other words -- should these remain to be called Ignition scripts, and let downstreams such as OpenShift convert to product-specific terms like Machine Configs? Trying to avoid having remediation snippets for OCP vs Docker vs Pivotal vs Rancher if possible, by letting Ignition be the commonality between them.

Copy link
Member

Choose a reason for hiding this comment

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

updated: chatted with @jhrozek and @JAORMX in person. Lose agreement that keeping ignition namespace might make sense as it will simplify code in the compliance operator.

@jhrozek double checking compliance-operator code and reporting back.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, if we were to keep this as ignition, we would then want to keep only from line 10 to 18. And remove the headers. Those should be handled by the compliance-operator as an implementation detail.

@shawndwells
Copy link
Member

@ggbecker Code was reviewed and jenkins passed - this can be merged.

To enable others to begin building remediation content this afternoon, what do you think about merging this now and having you update the namespace/naming back to ignition this afternoon?

If agreeable, feel free to create a ticket to track the work and self-merge.

@ggbecker
Copy link
Member Author

@ggbecker Code was reviewed and jenkins passed - this can be merged.

To enable others to begin building remediation content this afternoon, what do you think about merging this now and having you update the namespace/naming back to ignition this afternoon?

If agreeable, feel free to create a ticket to track the work and self-merge.

I agree. The code part which enables the capability to use a new type of remediation is seems to correct. We just need to figure out what is the format of the remediation, eg. which kind of data is expected to be there.

@ggbecker ggbecker merged commit 2b5f049 into ComplianceAsCode:master Jan 28, 2020
@ggbecker ggbecker added the Highlight This PR/Issue should make it to the featured changelog. label Jan 30, 2020
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Highlight This PR/Issue should make it to the featured changelog.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants