-
Notifications
You must be signed in to change notification settings - Fork 2k
OCPQE-31342: Update aggregated-cert post check #72963
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
base: master
Are you sure you want to change the base?
Conversation
|
Skipping CI for Draft Pull Request. |
|
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.21-amd64-nightly-azure-aks-hypershift-full-cert-guest-f14 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: xiuwang The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@xiuwang: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.21-amd64-nightly-azure-aks-hypershift-full-cert-guest-f14 |
|
@xiuwang: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.21-amd64-nightly-azure-aks-hypershift-full-cert-guest-f14 |
|
@xiuwang: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.20-amd64-nightly-azure-aks-hypershift-full-cert-guest-f14 |
|
@xiuwang: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.19-amd64-nightly-azure-aks-hypershift-full-cert-guest-f14 |
|
@xiuwang: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@xiuwang: This pull request references OCPQE-31342 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
lunarwhite
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your patch fix!
| # Get Let's Encrypt certificate chain from the custom serving cert secret | ||
| LETSENCRYPT_CERT="$(mgmt oc get secret -n clusters "$AGGREGATED_CERT_SECRET_NAME" -o jsonpath='{.data.tls\.crt}')" | ||
|
|
||
| # Combine CAs: OpenShift root CA + Let's Encrypt cert chain | ||
| COMBINED_CA_DATA="$(echo -e "${CURRENT_CA_DATA}\n${LETSENCRYPT_CERT}" | base64 -d | base64 | tr -d '\n')" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies I have minor knowledge in Hypershift, I have following concerns IMHO:
- Certificates issued by the Let's Encrypt are publicly trusted, so clients should already trust them without adding them to kubeconfig. If something is broken without this combination step, perhaps we should fix it elsewhere rather than relying on this workaround?
- The certs stored in
.data.tls.crtinclude the server certificate, which should not be included in the combined CA bundle used for TLS verification.
There is another existing TODO (might be relevant / out-of-scope):
Lines 186 to 188 in 343cf10
| # Update KUBECONFIG to allow secure communication between kubelets and the external KAS endpoint | |
| # TODO: remove this workaround once https://issues.redhat.com/browse/OCPBUGS-41853 is resolved | |
| remove_kubelet_kubeconfig_cluster_ca |
Just checked OCPBUGS-41853 now is resolved. Not sure whether removing the old outdated workaround would help simplify what this PR is trying to fix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OCPBUGS-41853 fixed as a document update, OCPSTRAT-1516 add a new approach to add external dns with cert , qe prow ci added in https://github.com/openshift/release/pull/64900/files.
Maybe I can configure to deprecated azure-aks-hypershift-full-cert-guest-f14 jobs
|
@xiuwang: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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-sigs/prow repository. I understand the commands that are listed here. |
Resolved: