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

Fix timestamps for data-stream composition #12625

Merged
merged 3 commits into from
Feb 5, 2025

Conversation

ggbecker
Copy link
Member

Description:

  • Unify and use the constant timestamp across the whole code.

  • Previously oscap info command would print null for such cases. Stream: scap_org.open-scap_datastream_from_xccdf_ssg-fedora-xccdf.xml Generated: (null)

Review Hints:

  • oscap info "ds.file"

Previous:
~ 0 $ oscap info "/usr/share/xml/scap/ssg/content/ssg-fedora-ds.xml"
Document type: Source Data Stream
Imported: 2024-08-12T02:00:00

Stream: scap_org.open-scap_datastream_from_xccdf_ssg-fedora-xccdf.xml
Generated: (null)
Version: 1.3
Checklists:

Current:
oscap info build/ssg-rhel9-ds.xml
Document type: Source Data Stream
Imported: 2024-11-21T14:20:20

Stream: scap_org.open-scap_datastream_from_xccdf_ssg-rhel9-xccdf.xml
Generated: 2024-11-21T13:20:19
Version: 1.3
Checklists:

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

@ggbecker ggbecker added this to the 0.1.76 milestone Nov 22, 2024
@jan-cerny jan-cerny changed the title Fix timestamps for data-stream composition. Fix timestamps for data-stream composition Nov 26, 2024
@jan-cerny jan-cerny self-assigned this Nov 28, 2024
Copy link
Collaborator

@jan-cerny jan-cerny left a comment

Choose a reason for hiding this comment

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

@ggbecker What do you think about the date attribute of the status element within the Benchmark element? I think this one should be changed as well because it depends on the current date. See build_yaml.py line 426.

@jan-cerny
Copy link
Collaborator

@ggbecker What is your opinion on the comment above?

@ggbecker
Copy link
Member Author

@ggbecker What do you think about the date attribute of the status element within the Benchmark element? I think this one should be changed as well because it depends on the current date. See build_yaml.py line 426.

This got fixed in this commit: d561c6a

But I believe we could still use the timestamp variable there, the only thing I don't know is if the time format is a problem since the status is now being populated only by the day/month/year, opposed to the day/month/year + hh:mm:ss coming from the timestamp variable. What do you think?

@jan-cerny
Copy link
Collaborator

Yes, the format seems to be a problem, because the schema validation fails. The status element needs to contain date in a form of YYYY-DD-MM, for example: <xccdf-1.2:status date="2024-12-03">draft</xccdf-1.2:status>. But the date really needs to depend on the timestamp variable otherwise the build won't be reproducible. So I suggest writing some formatting code that takes the timestamp, creates the date in a proper format and inserts it into the status element.

Unify and use the constant timestamp across the whole code.

Previously oscap info command would print null for such cases.
Stream: scap_org.open-scap_datastream_from_xccdf_ssg-fedora-xccdf.xml
Generated: (null)
This constant derives from the same timestamp used across everywhere but
uses a slightly different format YYYY-mm-dd so it passes XCCDF
validation.
@ggbecker
Copy link
Member Author

ggbecker commented Feb 4, 2025

Yes, the format seems to be a problem, because the schema validation fails. The status element needs to contain date in a form of YYYY-DD-MM, for example: <xccdf-1.2:status date="2024-12-03">draft</xccdf-1.2:status>. But the date really needs to depend on the timestamp variable otherwise the build won't be reproducible. So I suggest writing some formatting code that takes the timestamp, creates the date in a proper format and inserts it into the status element.

It should be now resolved, let's see how the CI goes.

Copy link

codeclimate bot commented Feb 4, 2025

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

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

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

View more on Code Climate.

@vojtapolasek vojtapolasek modified the milestones: 0.1.76, 0.1.77 Feb 4, 2025
Copy link
Collaborator

@jan-cerny jan-cerny left a comment

Choose a reason for hiding this comment

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

I have reviewed the timestamp attributes in the built data stream both in with and without the SOURCE_DATE_EPOCH variable.

@jan-cerny jan-cerny merged commit 512a2dc into ComplianceAsCode:master Feb 5, 2025
109 checks passed
@jan-cerny jan-cerny added the Infrastructure Our content build system label Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Infrastructure Our content build system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants