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

Modify API Resource Collector to Convert YAML to JSON Format #235

Merged
merged 1 commit into from
Mar 23, 2023

Conversation

Vincent056
Copy link

@Vincent056 Vincent056 commented Feb 28, 2023

Modify API resource collector to detect and convert fetched resources to json format when they are returned in yaml format. This is required because some API resources are not available in json format and need to be converted to json format for reading by OpenSCAP.

example rules:

applications/openshift/api-server/ocp_api_server_audit_log_maxsize
applications/openshift/api-server/ocp_api_server_audit_log_maxbackup

CHANGELOG.md Outdated
@@ -9,7 +9,10 @@ Versioning](https://semver.org/spec/v2.0.0.html).

### Enhancements

-
- Modify API resource collector to detect if fetched resource is yaml string and
Copy link

Choose a reason for hiding this comment

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

My only complaint about this patch is that this should not be an enhancement but either a fix or even an internal change.

Copy link
Author

Choose a reason for hiding this comment

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

That makes sense, let me do that

Copy link

Choose a reason for hiding this comment

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

@Vincent056 if you move the changelog line, I'll lgtm the PR :-)

@jhrozek
Copy link

jhrozek commented Mar 2, 2023

btw this PR should be linked to https://issues.redhat.com/browse/OCPBUGS-7983 right?

@jhrozek
Copy link

jhrozek commented Mar 2, 2023

btw this PR should be linked to https://issues.redhat.com/browse/OCPBUGS-7983 right?

If yes, could we fix the changelog?

@Vincent056
Copy link
Author

btw this PR should be linked to https://issues.redhat.com/browse/OCPBUGS-7983 right?

the jira issue you referenced here was the result of not using a correct tailored profile.

Without this patch, those two rules will just fail but not result in pod crash.

@jhrozek
Copy link

jhrozek commented Mar 3, 2023

OK, then I'll let @xiaojiey confirm if the patch + TP fix her issues (I did confirm that the crash is fixed)

@xiaojiey
Copy link
Collaborator

/hold for test

@xiaojiey
Copy link
Collaborator

xiaojiey commented Mar 14, 2023

Verification pass with #235 and ComplianceAsCode/content#10333.
From the test result, api-server-audit-log-maxbackup will PASS and api-server-audit-log-maxbackup will FAIL.
Per below command, it is working as expected.

$ oc get --raw /api/v1/namespaces/clusters-hypershift-ci-32558/configmaps/openshift-apiserver | jq '.data."config.yaml"' | jq -r | grep apiServerArguments -A 10
apiServerArguments:
  audit-log-format:
  - json
  audit-log-maxsize:
  - "100"
  audit-log-path:
  - /var/log/openshift-apiserver/audit.log
  audit-policy-file:
  - /etc/kubernetes/audit-config/policy.yaml
  shutdown-delay-duration:
  - 3s

Verification steps:

1. Install Complinace Operator will code in this PR.
2. Build content with pr https://github.com/ComplianceAsCode/content/pull/10333
3. Create a tp:
$ oc apply -f -<<EOF
> apiVersion: compliance.openshift.io/v1alpha1
kind: TailoredProfile
metadata:
 name: pcidss-compliance-hypershift
 namespace: openshift-compliance
 annotations:
   compliance.openshift.io/product-type: Platform
spec:
 title: PCI-DSS Benchmark for Hypershift
 description: PCI-DSS Benchmark for Hypershift Master-plane components
 extends: upstream-ocp4-pci-dss
 setValues:
   - name: upstream-ocp4-hypershift-cluster
     value: "hypershift-ci-32558"
     rationale: This value is used for HyperShift version detection, and also to determine the namespace of the hosted cluster
EOF
4. Create a ssb and check result:
$ oc compliance bind -N test -S default tailoredprofile/pcidss-compliance-hypershift
Creating ScanSettingBinding test
$ oc get suite -w
NAME   PHASE     RESULT
test   RUNNING   NOT-AVAILABLE
test   AGGREGATING   NOT-AVAILABLE
test   DONE          NON-COMPLIANT
test   DONE          NON-COMPLIANT
^C$ oc get ccr | grep maxsize
pcidss-compliance-hypershift-api-server-audit-log-maxsize                              PASS     medium
pcidss-compliance-hypershift-ocp-api-server-audit-log-maxsize                          PASS     medium
$ oc get ccr | grep maxbackup
pcidss-compliance-hypershift-api-server-audit-log-maxbackup                            PASS     low
pcidss-compliance-hypershift-ocp-api-server-audit-log-maxbackup                        FAIL     low
$ $ oc get ccr -l compliance.openshift.io/check-status=FAIL
NAME                                                                 STATUS   SEVERITY
pcidss-compliance-hypershift-audit-log-forwarding-enabled            FAIL     medium
pcidss-compliance-hypershift-configure-network-policies-namespaces   FAIL     high
pcidss-compliance-hypershift-file-integrity-exists                   FAIL     medium
pcidss-compliance-hypershift-file-integrity-notification-enabled     FAIL     medium
pcidss-compliance-hypershift-kubeadmin-removed                       FAIL     medium
pcidss-compliance-hypershift-ocp-api-server-audit-log-maxbackup                        FAIL     low

@xiaojiey
Copy link
Collaborator

/unhold

@xiaojiey
Copy link
Collaborator

/label qe-approved

@Vincent056
Copy link
Author

/retest

@Vincent056
Copy link
Author

/retest

1 similar comment
@Vincent056
Copy link
Author

/retest

@jhrozek
Copy link

jhrozek commented Mar 21, 2023

welp, there's already a conflict, can you rebase again?

Modify API resource collector to detect and convert fetched resources to json format when they are returned in yaml format. This is required because some API resources are not available in json format and need to be converted to json format for reading by OpenSCAP.
Copy link

@jhrozek jhrozek left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci
Copy link

openshift-ci bot commented Mar 22, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jhrozek, Vincent056

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit 5712197 into ComplianceAsCode:master Mar 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants