Use `string` instead of `number` in oauth variable #11613

rhmdnd · 2024-02-21T20:30:44Z

Recent changes to the oauth token maxage variable introduced it as a
number instead of a string or integer, which SCAP is expecting. This
commit updates the variable to be an int instead of a number type, so
that the resulting datastream is valid and parsable by the compliance
oeprator profile parsers.

Without this change, content from the latest branch will cause the
compliance operator installs to hang because it can't parse the xccdf.

rhmdnd · 2024-02-21T20:30:55Z

/test

openshift-ci · 2024-02-21T20:31:03Z

@rhmdnd: The /test command needs one or more targets.
The following commands are available to trigger required jobs:

/test 4.13-e2e-aws-ocp4-cis
/test 4.13-e2e-aws-ocp4-cis-node
/test 4.13-e2e-aws-ocp4-e8
/test 4.13-e2e-aws-ocp4-high
/test 4.13-e2e-aws-ocp4-high-node
/test 4.13-e2e-aws-ocp4-moderate
/test 4.13-e2e-aws-ocp4-moderate-node
/test 4.13-e2e-aws-ocp4-pci-dss
/test 4.13-e2e-aws-ocp4-pci-dss-node
/test 4.13-e2e-aws-ocp4-stig
/test 4.13-e2e-aws-ocp4-stig-node
/test 4.13-e2e-aws-rhcos4-e8
/test 4.13-e2e-aws-rhcos4-high
/test 4.13-e2e-aws-rhcos4-moderate
/test 4.13-e2e-aws-rhcos4-stig
/test 4.13-images
/test 4.14-images
/test 4.15-e2e-aws-ocp4-cis
/test 4.15-e2e-aws-ocp4-cis-node
/test 4.15-e2e-aws-ocp4-e8
/test 4.15-e2e-aws-ocp4-high
/test 4.15-e2e-aws-ocp4-high-node
/test 4.15-e2e-aws-ocp4-moderate
/test 4.15-e2e-aws-ocp4-moderate-node
/test 4.15-e2e-aws-ocp4-pci-dss
/test 4.15-e2e-aws-ocp4-pci-dss-node
/test 4.15-e2e-aws-ocp4-stig
/test 4.15-e2e-aws-ocp4-stig-node
/test 4.15-e2e-aws-rhcos4-e8
/test 4.15-e2e-aws-rhcos4-high
/test 4.15-e2e-aws-rhcos4-moderate
/test 4.15-e2e-aws-rhcos4-stig
/test 4.15-images
/test 4.16-e2e-aws-ocp4-cis
/test 4.16-e2e-aws-ocp4-cis-node
/test 4.16-e2e-aws-ocp4-e8
/test 4.16-e2e-aws-ocp4-high
/test 4.16-e2e-aws-ocp4-high-node
/test 4.16-e2e-aws-ocp4-moderate
/test 4.16-e2e-aws-ocp4-moderate-node
/test 4.16-e2e-aws-ocp4-pci-dss
/test 4.16-e2e-aws-ocp4-pci-dss-node
/test 4.16-e2e-aws-ocp4-stig
/test 4.16-e2e-aws-ocp4-stig-node
/test 4.16-e2e-aws-rhcos4-e8
/test 4.16-e2e-aws-rhcos4-high
/test 4.16-e2e-aws-rhcos4-moderate
/test 4.16-e2e-aws-rhcos4-stig
/test 4.16-images
/test e2e-aws-ocp4-cis
/test e2e-aws-ocp4-cis-node
/test e2e-aws-ocp4-e8
/test e2e-aws-ocp4-high
/test e2e-aws-ocp4-high-node
/test e2e-aws-ocp4-moderate
/test e2e-aws-ocp4-moderate-node
/test e2e-aws-ocp4-pci-dss
/test e2e-aws-ocp4-pci-dss-node
/test e2e-aws-ocp4-stig
/test e2e-aws-ocp4-stig-node
/test e2e-aws-rhcos4-e8
/test e2e-aws-rhcos4-high
/test e2e-aws-rhcos4-moderate
/test e2e-aws-rhcos4-stig
/test images

Use /test all to run the following jobs that were automatically triggered:

pull-ci-ComplianceAsCode-content-master-4.13-images
pull-ci-ComplianceAsCode-content-master-4.14-images
pull-ci-ComplianceAsCode-content-master-4.15-images
pull-ci-ComplianceAsCode-content-master-4.16-images
pull-ci-ComplianceAsCode-content-master-images

In response to this:

/test

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.

rhmdnd · 2024-02-21T20:32:01Z

/test 4.15-e2e-aws-ocp4-stig
/test e2e-aws-ocp4-stig
/test 4.16-e2e-aws-ocp4-stig
/test 4.13-e2e-aws-ocp4-stig

github-actions · 2024-02-21T20:32:09Z

Start a new ephemeral environment with changes proposed in this pull request:

Fedora Environment

Oracle Linux 8 Environment

github-actions · 2024-02-21T20:35:27Z

🤖 A k8s content image for this PR is available at:
ghcr.io/complianceascode/k8scontent:11613

Click here to see how to deploy it

If you alread have Compliance Operator deployed:
utils/build_ds_container.py -i ghcr.io/complianceascode/k8scontent:11613

Otherwise deploy the content and operator together by checking out ComplianceAsCode/compliance-operator and:
CONTENT_IMAGE=ghcr.io/complianceascode/k8scontent:11613 make deploy-local

rhmdnd · 2024-02-21T20:36:43Z

/test 4.15-e2e-aws-ocp4-stig
/test e2e-aws-ocp4-stig
/test 4.16-e2e-aws-ocp4-stig
/test 4.13-e2e-aws-ocp4-stig

rhmdnd · 2024-02-21T20:37:36Z

/test 4.15-e2e-aws-ocp4-stig
/test e2e-aws-ocp4-stig
/test 4.16-e2e-aws-ocp4-stig
/test 4.13-e2e-aws-ocp4-stig

rhmdnd · 2024-02-21T22:00:11Z

/test 4.15-e2e-aws-ocp4-stig
/test e2e-aws-ocp4-stig
/test 4.16-e2e-aws-ocp4-stig
/test 4.13-e2e-aws-ocp4-stig

rhmdnd · 2024-02-21T22:06:29Z

Rekicking CI since I think the original change was using the wrong type still.

Mab879 · 2024-02-21T22:11:08Z

subprocess.CalledProcessError: Command '['oscap', 'xccdf', 'generate', 'guide', '--benchmark-id', 'xccdf_org.ssgproject.content_benchmark_OCP-4', '--profile', 'xccdf_org.ssgproject.content_profile_bsi-node', '/__w/content/content/build/ssg-ocp4-ds.xml']' returned non-zero exit status 1.
OpenSCAP Error: File '/__w/content/content/build/ssg-ocp4-ds.xml' line 7301: Element '{http://checklists.nist.gov/xccdf/1.2}Value', attribute 'type': [facet 'enumeration'] The value 'int' is not an element of the set {'number', 'string', 'boolean'}.

I think string will be the needed type.

rhmdnd · 2024-02-21T23:14:24Z

/test 4.15-e2e-aws-ocp4-stig
/test e2e-aws-ocp4-stig
/test 4.16-e2e-aws-ocp4-stig
/test 4.13-e2e-aws-ocp4-stig

rhmdnd · 2024-02-22T01:36:57Z

Waiting to get CI results back on another approach

#11614

yuumasato · 2024-02-22T09:37:47Z

This may not be what is causing the profile parsing problems.
Regardless, this change makes sense, as both rules oauth_token_maxage and oauthclient_token_maxage generate the check with string type.
And we do not check whether one value is higher than the other, using string type should be enough when checking for equal values.

rhmdnd · 2024-02-22T13:34:45Z

This may not be what is causing the profile parsing problems. Regardless, this change makes sense, as both rules oauth_token_maxage and oauthclient_token_maxage generate the check with string type. And we do not check whether one value is higher than the other, using string type should be enough when checking for equal values.

Thanks for confirming.

Even if they can be string type, we have other variables that use strings for integers (e.g., "600").

Are there other advantages or disadvantages to only using strings for everything, even if it's ultimately an integer?

yuumasato · 2024-02-22T13:47:30Z

Are there other advantages or disadvantages to only using strings for everything, even if it's ultimately an integer?

I think the only difference will be on the type of comparison/checking we want to with the value.
With strings we can do equals and pattern match;
while with int we can do equals, greater than and less than.

Here is the complete list of possible operations:
https://oval-community-guidelines.readthedocs.io/en/latest/oval-schema-documentation/oval-common-schema.html?highlight=pattern%20match#simpledatatypeenumeration-1

Vincent056 · 2024-02-22T21:03:17Z

I think string will be the needed type.

@Mab879 I wonder if you know why it is complaining that we have int when we have number as the type.

Mab879 · 2024-02-22T22:23:44Z

I think string will be the needed type.

@Mab879 I wonder if you know why it is complaining that we have int when we have number as the type.

I don't have any theories.

xiaojiey · 2024-02-26T13:11:13Z

@rhmdnd I still got the CrashLoopBackOff for the profile parser pods. Maybe a rebase needed? Thanks.

xiaojiey · 2024-02-26T13:12:01Z

/hold for test

yuumasato · 2024-02-27T17:00:37Z

@rhmdnd I think this needs to be rebased to pick the encoding change

Recent changes to the oauth token maxage variable introduced it as a number instead of a string or integer, which SCAP is expecting. This commit updates the variable to be a string instead of a number type, so that the resulting datastream is valid and parsable by the compliance oeprator profile parsers. Without this change, content from the latest branch will cause the compliance operator installs to hang because it can't parse the xccdf.

rhmdnd · 2024-02-28T15:22:48Z

/test 4.15-e2e-aws-ocp4-stig
/test e2e-aws-ocp4-stig
/test 4.16-e2e-aws-ocp4-stig
/test 4.13-e2e-aws-ocp4-stig

codeclimate · 2024-02-28T16:33:47Z

Code Climate has analyzed commit a5a2782 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 57.8% (0.0% change).

View more on Code Climate.

xiaojiey · 2024-02-29T12:51:38Z

Verification pass with 4.16.0-0.nightly-2024-02-26-013420:

1. Verify string required for  variable ocp4-var-oauth-token-maxage:
$ cat tp.yaml 
#upstream-ocp4-oauthclient-token-maxage
apiVersion: compliance.openshift.io/v1alpha1
kind: TailoredProfile
metadata:
  name: example-tailoredprofile
spec:
  description: test
  extends: upstream-ocp4-moderate
  title: My little profile
  enableRules:
    - name: upstream-ocp4-oauth-token-maxage
      rationale: testing this
  setValues:
  - name: upstream-ocp4-var-oauth-token-maxage
    rationale: test
    value: 7200
$ oc apply -f tp.yaml 
The TailoredProfile "example-tailoredprofile" is invalid: spec.setValues.value: Required string
$ cat tp.yaml 
#upstream-ocp4-oauthclient-token-maxage
apiVersion: compliance.openshift.io/v1alpha1
kind: TailoredProfile
metadata:
  name: example-tailoredprofile
spec:
  description: test
  extends: upstream-ocp4-moderate
  title: My little profile
  enableRules:
    - name: upstream-ocp4-oauth-token-maxage
      rationale: testing this
  setValues:
  - name: upstream-ocp4-var-oauth-token-maxage
    rationale: test
    value: "7200"
$ oc apply -f tp.yaml 
tailoredprofile.compliance.openshift.io/example-tailoredprofile created
2. With oauth token not equals the variable, the rule FAIL:
$ oc get oauth cluster -ojsonpath='{.spec}'
{"identityProviders":[{"htpasswd":{"fileData":{"name":"htpass-secret"}},"mappingMethod":"claim","name":"flexy-htpasswd-provider","type":"HTPasswd"}]}
$ oc compliance bind -N test tailoredprofile/example-tailoredprofile
Creating ScanSettingBinding test
$ oc get suite 
NAME   PHASE   RESULT
test   DONE    NON-COMPLIANT
$ oc get ccr | grep maxage
example-tailoredprofile-oauth-or-oauthclient-token-maxage                         FAIL     medium
example-tailoredprofile-oauth-token-maxage                                        FAIL     medium
3. When oauth token equals the variable, the rule PASS:
$ oc patch oauth cluster -p '{"spec":{"tokenConfig":{"accessTokenMaxAgeSeconds":7200}}}' --type=merge
oauth.config.openshift.io/cluster patched
$ oc compliance rerun-now scansettingbinding test
Rerunning scans from 'test': example-tailoredprofile
Re-running scan 'openshift-compliance/example-tailoredprofile'
$ oc get ccr | grep maxage
example-tailoredprofile-oauth-or-oauthclient-token-maxage                         PASS     medium
example-tailoredprofile-oauth-token-maxage                                        PASS     medium

xiaojiey · 2024-02-29T12:51:47Z

/unhold

yuumasato

number is not an invalid type, but given the extended rule definitions, string is better suited.

rhmdnd requested a review from yuumasato February 21, 2024 20:30

rhmdnd added the OpenShift OpenShift product related. label Feb 21, 2024

rhmdnd force-pushed the fix-invalid-number-variable-usage branch from d4b4790 to 3cb1249 Compare February 21, 2024 20:35

rhmdnd force-pushed the fix-invalid-number-variable-usage branch from 3cb1249 to eeed8a6 Compare February 21, 2024 20:37

Mab879 self-assigned this Feb 21, 2024

Mab879 added this to the 0.1.73 milestone Feb 21, 2024

rhmdnd force-pushed the fix-invalid-number-variable-usage branch from eeed8a6 to 057e0e2 Compare February 21, 2024 23:14

rhmdnd added the do-not-merge/hold Used by openshift-ci-robot bot. label Feb 22, 2024

Mab879 removed their assignment Feb 22, 2024

rhmdnd force-pushed the fix-invalid-number-variable-usage branch from 057e0e2 to a5a2782 Compare February 28, 2024 15:22

openshift-ci bot removed the do-not-merge/hold Used by openshift-ci-robot bot. label Feb 29, 2024

yuumasato self-assigned this Feb 29, 2024

yuumasato changed the title ~~Fix invalid number usage in oauth variable~~ Use string instead of number in oauth variable Feb 29, 2024

yuumasato approved these changes Feb 29, 2024

View reviewed changes

yuumasato merged commit eb2f364 into ComplianceAsCode:master Feb 29, 2024
48 checks passed

Mab879 added the Update Rule Issues or pull requests related to Rules updates. label May 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use `string` instead of `number` in oauth variable #11613

Use `string` instead of `number` in oauth variable #11613

rhmdnd commented Feb 21, 2024 •

edited

Loading

rhmdnd commented Feb 21, 2024

openshift-ci bot commented Feb 21, 2024

rhmdnd commented Feb 21, 2024

github-actions bot commented Feb 21, 2024

github-actions bot commented Feb 21, 2024

rhmdnd commented Feb 21, 2024

rhmdnd commented Feb 21, 2024

rhmdnd commented Feb 21, 2024

rhmdnd commented Feb 21, 2024

Mab879 commented Feb 21, 2024

rhmdnd commented Feb 21, 2024

rhmdnd commented Feb 22, 2024

yuumasato commented Feb 22, 2024

rhmdnd commented Feb 22, 2024

yuumasato commented Feb 22, 2024 •

edited

Loading

Vincent056 commented Feb 22, 2024

Mab879 commented Feb 22, 2024

xiaojiey commented Feb 26, 2024

xiaojiey commented Feb 26, 2024

yuumasato commented Feb 27, 2024

rhmdnd commented Feb 28, 2024

codeclimate bot commented Feb 28, 2024

xiaojiey commented Feb 29, 2024 •

edited

Loading

xiaojiey commented Feb 29, 2024

yuumasato left a comment •

edited

Loading

Use string instead of number in oauth variable #11613

Use string instead of number in oauth variable #11613

Conversation

rhmdnd commented Feb 21, 2024 • edited Loading

rhmdnd commented Feb 21, 2024

openshift-ci bot commented Feb 21, 2024

rhmdnd commented Feb 21, 2024

github-actions bot commented Feb 21, 2024

github-actions bot commented Feb 21, 2024

rhmdnd commented Feb 21, 2024

rhmdnd commented Feb 21, 2024

rhmdnd commented Feb 21, 2024

rhmdnd commented Feb 21, 2024

Mab879 commented Feb 21, 2024

rhmdnd commented Feb 21, 2024

rhmdnd commented Feb 22, 2024

yuumasato commented Feb 22, 2024

rhmdnd commented Feb 22, 2024

yuumasato commented Feb 22, 2024 • edited Loading

Vincent056 commented Feb 22, 2024

Mab879 commented Feb 22, 2024

xiaojiey commented Feb 26, 2024

xiaojiey commented Feb 26, 2024

yuumasato commented Feb 27, 2024

rhmdnd commented Feb 28, 2024

codeclimate bot commented Feb 28, 2024

xiaojiey commented Feb 29, 2024 • edited Loading

xiaojiey commented Feb 29, 2024

yuumasato left a comment • edited Loading

Choose a reason for hiding this comment

Use `string` instead of `number` in oauth variable #11613

Use `string` instead of `number` in oauth variable #11613

rhmdnd commented Feb 21, 2024 •

edited

Loading

yuumasato commented Feb 22, 2024 •

edited

Loading

xiaojiey commented Feb 29, 2024 •

edited

Loading

yuumasato left a comment •

edited

Loading