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

OCP4: use utf-8 as default xml encoding #11614

Merged
merged 1 commit into from
Feb 22, 2024

Conversation

Vincent056
Copy link
Contributor

@Vincent056 Vincent056 commented Feb 22, 2024

This commit make UTF-8 as default xml encoding. In a recent change to the build system, xml encoding was changed from not specific(UTF-8) to US-ASCII .

When writing XCCDF XML file, python xml uses the output encoding US-ASCII as default if not set, however this causes issues in Compliance Operator parser, using US-ASCII, the parser is not able to get the prefix of xml node, therefore crash the parser pod.

snippet of XCCDF print out using utf-8

 <?xml version="1.0" encoding="utf-8"?>

parsed into

ds:data-stream-collection
ds:data-stream-collection/ds:data-stream
ds:data-stream-collection/ds:data-stream/ds:dictionaries
ds:data-stream-collection/ds:data-stream/ds:dictionaries/ds:component-ref
ds:data-stream-collection/ds:data-stream/ds:dictionaries/ds:component-ref/cat:catalog
ds:data-stream-collection/ds:data-stream/ds:dictionaries/ds:component-ref/cat:catalog/cat:uri
ds:data-stream-collection/ds:data-stream/ds:checklists
ds:data-stream-collection/ds:data-stream/ds:checklists/ds:component-ref
ds:data-stream-collection/ds:data-stream/ds:checklists/ds:component-ref/cat:catalog
ds:data-stream-collection/ds:data-stream/ds:checklists/ds:component-ref/cat:catalog/cat:uri
ds:data-stream-collection/ds:data-stream/ds:checks
ds:data-stream-collection/ds:data-stream/ds:checks/ds:component-ref

snippet of XCCDF print out using us-ascii parsed into:

<?xml version="1.0" encoding="us-ascii"?>
data-stream-collection
data-stream-collection/data-stream
data-stream-collection/data-stream/dictionaries
data-stream-collection/data-stream/dictionaries/component-ref
data-stream-collection/data-stream/dictionaries/component-ref/catalog
data-stream-collection/data-stream/dictionaries/component-ref/catalog/uri
data-stream-collection/data-stream/checklists
data-stream-collection/data-stream/checklists/component-ref
data-stream-collection/data-stream/checklists/component-ref/catalog
data-stream-collection/data-stream/checklists/component-ref/catalog/uri
data-stream-collection/data-stream/checks
data-stream-collection/data-stream/checks/component-ref

This commit make UTF-8 as default xml encoding. In a recent change, when writing xccdf xml file, it uses the output encoding US-ASCII as default if not set, however this causes issues in Compliance Operator parser, using US-ASCII, the parser is not able to get the prefix of xml node, therefore crash the parser pod.
Copy link

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

Fedora Environment
Open in Gitpod

Oracle Linux 8 Environment
Open in Gitpod

@Vincent056
Copy link
Contributor Author

/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

Copy link

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

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:11614

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

Copy link

codeclimate bot commented Feb 22, 2024

Code Climate has analyzed commit ace1a53 and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Duplication 1

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

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

View more on Code Climate.

@Vincent056
Copy link
Contributor Author

AWS quota hit

@Vincent056
Copy link
Contributor Author

/test e2e-aws-ocp4-stig

@Vincent056
Copy link
Contributor Author

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

@jan-cerny
Copy link
Collaborator

/packit build

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.

LGTM.

Pushing the thin DS PR content image results in CrashLoopBackOff:
$ utils/build_ds_container.py -i ghcr.io/complianceascode/k8scontent:11549

But this PR's image works fine:
$ utils/build_ds_container.py -i ghcr.io/complianceascode/k8scontent:11614

@yuumasato
Copy link
Member

@jan-cerny @Honny1 Do you guys have any objection to this change?

@Honny1
Copy link
Collaborator

Honny1 commented Feb 22, 2024

@yuumasato I have no objection.

@xiaojiey
Copy link
Collaborator

/lgtm

Copy link
Collaborator

@rhmdnd rhmdnd left a comment

Choose a reason for hiding this comment

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

/lgtm

@yuumasato yuumasato added the OpenShift OpenShift product related. label Feb 22, 2024
@yuumasato yuumasato added this to the 0.1.73 milestone Feb 22, 2024
@yuumasato yuumasato merged commit 32f41d8 into ComplianceAsCode:master Feb 22, 2024
45 of 48 checks passed
@Mab879 Mab879 added the Infrastructure Our content build system label May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Infrastructure Our content build system OpenShift OpenShift product related.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants