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

[edpm_image_build] Copy certificates based on regexes #2673

Merged
merged 1 commit into from
Jan 23, 2025

Conversation

pablintino
Copy link
Collaborator

To avoid assuming the source certificate to copy into the diskimage-builder has a specific name just use the system CAs chain and allow the user to provide some regexes to select which CAs should be copied.

@pablintino pablintino force-pushed the rework-image-builder-certs-copy-logic branch 3 times, most recently from 91053cc to 581bc52 Compare January 21, 2025 15:23
Copy link

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/91e7a27003d84220a087735093d190dc

✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 39m 27s
✔️ podified-multinode-edpm-deployment-crc SUCCESS in 1h 12m 36s
✔️ cifmw-crc-podified-edpm-baremetal SUCCESS in 1h 19m 52s
✔️ noop SUCCESS in 0s
cifmw-pod-ansible-test FAILURE in 5m 48s
✔️ cifmw-pod-pre-commit SUCCESS in 7m 24s
✔️ cifmw-content-provider-build-images SUCCESS in 23m 26s
✔️ cifmw-edpm-build-images SUCCESS in 22m 20s
✔️ ci-framework-openstack-meta-content-provider SUCCESS in 13m 05s
✔️ build-push-container-cifmw-client SUCCESS in 23m 20s
✔️ cifmw-molecule-edpm_build_images SUCCESS in 5m 15s

@pablintino pablintino force-pushed the rework-image-builder-certs-copy-logic branch from 581bc52 to 199b359 Compare January 21, 2025 23:11
Copy link

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/0f9726fc52c2422280e4878d9dd28879

✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 43m 01s
✔️ podified-multinode-edpm-deployment-crc SUCCESS in 1h 13m 34s
✔️ cifmw-crc-podified-edpm-baremetal SUCCESS in 1h 29m 22s
✔️ noop SUCCESS in 0s
cifmw-pod-ansible-test FAILURE in 5m 28s
cifmw-pod-pre-commit FAILURE in 7m 19s
✔️ cifmw-content-provider-build-images SUCCESS in 14m 50s
✔️ cifmw-edpm-build-images SUCCESS in 22m 02s
ci-framework-openstack-meta-content-provider FAILURE in 12m 15s
✔️ build-push-container-cifmw-client SUCCESS in 21m 58s
✔️ cifmw-molecule-edpm_build_images SUCCESS in 5m 24s

@pablintino pablintino force-pushed the rework-image-builder-certs-copy-logic branch 4 times, most recently from 1909102 to e90b63d Compare January 22, 2025 10:34
@lewisdenny
Copy link
Collaborator

/approve

Copy link
Contributor

openshift-ci bot commented Jan 22, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lewisdenny

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

plugins/modules/pem_read.py Outdated Show resolved Hide resolved
plugins/modules/pem_read.py Show resolved Hide resolved
plugins/modules/pem_read.py Outdated Show resolved Hide resolved
@evallesp
Copy link

Minor commit suggestion. LGTM!

To avoid assuming the source certificate to copy into the
diskimage-builder has a specific name just use the system CAs chain and
allow the user to provide some regexes to select which CAs should be
copied.

The new module can read a PEM file (that can list many certs) and,
optionally, filter the certs by OU or CN regexes.
@pablintino pablintino force-pushed the rework-image-builder-certs-copy-logic branch from e90b63d to 04d7424 Compare January 23, 2025 10:35
Copy link
Collaborator

@frenzyfriday frenzyfriday left a comment

Choose a reason for hiding this comment

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

/lgtm

I did not know about the ldap filters (so I googled that). Will we need to add more filters like dc_filter later on (I am not sure if that is even a filter - just saw it somewhere)?

@bshewale
Copy link
Contributor

Do we have coverage to test this change because i have seen the jobs ran in the tide here https://logserver.rdoproject.org/73/2673/04d74241fbc5700accd07880c8cc424aa0bd97d4/github-check/cifmw-edpm-build-images/404ed41/job-output.txt but it's not testing the change as may be the when condition has been false.

@pablintino
Copy link
Collaborator Author

pablintino commented Jan 23, 2025

Do we have coverage to test this change because i have seen the jobs ran in the tide here https://logserver.rdoproject.org/73/2673/04d74241fbc5700accd07880c8cc424aa0bd97d4/github-check/cifmw-edpm-build-images/404ed41/job-output.txt but it's not testing the change as may be the when condition has been false.

Yeah, in upstream the entire yaml file is not executed. We do need it in DS, but the jobs are already blocked, thus this change.
About coverage, there's a test to test the module itself that should catch almost all the python module related faults.

@pablintino
Copy link
Collaborator Author

/lgtm

I did not know about the ldap filters (so I googled that). Will we need to add more filters like dc_filter later on (I am not sure if that is even a filter - just saw it somewhere)?

Oh, what a casualty in naming, this has nothing to do with LDAP DNs but with x509 certificates.

@openshift-merge-bot openshift-merge-bot bot merged commit c6ada1f into main Jan 23, 2025
5 checks passed
@openshift-merge-bot openshift-merge-bot bot deleted the rework-image-builder-certs-copy-logic branch January 23, 2025 13:00
pablintino added a commit to pablintino/ci-framework that referenced this pull request Jan 29, 2025
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.

5 participants