OCPBUGS-44842: Set not-before/not-after annotations #1873

vrutkovs · 2025-07-24T07:01:00Z

Pull in openshift/library-go#1971 to make sure most secrets created by controller would have refresh-period annotation set.

openshift-ci · 2025-07-24T07:01:05Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

openshift-ci-robot · 2025-07-24T07:01:06Z

@vrutkovs: This pull request references Jira Issue OCPBUGS-57049, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.20.0) matches configured target version for branch (4.20.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @wangke19

The bug has been updated to refer to the pull request using the external bug tracker.

In 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.

vrutkovs · 2025-07-24T07:01:13Z

/test e2e-aws-ovn

vrutkovs · 2025-07-24T07:34:33Z

/test e2e-aws-ovn

vrutkovs · 2025-07-24T08:05:43Z

/test e2e-aws-ovn

vrutkovs · 2025-07-24T10:27:34Z

/test e2e-aws-ovn

vrutkovs · 2025-07-24T12:59:45Z

/cc @p0lyn0mial

openshift-ci-robot · 2025-07-24T13:07:21Z

@vrutkovs: This pull request references Jira Issue OCPBUGS-44842, which is invalid:

expected the bug to be in one of the following states: NEW, ASSIGNED, POST, but it is Verified instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

Pull in openshift/library-go#1971 to make sure most secrets created by controller would have refresh-period annotation set.

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.

vrutkovs · 2025-07-24T13:12:26Z

/jira refresh

wangke19 · 2025-07-28T10:52:27Z

Found minor typo in original code,
infrastuctureLister configv1listers.InfrastructureLister
Typo in infrastuctureLister → should be infrastructureLister (spelling consistency)

vrutkovs · 2025-07-28T11:02:14Z

good catch, fixed it

p0lyn0mial

LGTM, added some minor comments. thanks.

pkg/operator/nodekubeconfigcontroller/nodekubeconfigcontroller_test.go

infrastuctureLister -> infrastructureLister

wangke19 · 2025-07-29T07:57:26Z

/retest-required

p0lyn0mial

lgtm, just one more question.

pkg/operator/nodekubeconfigcontroller/nodekubeconfigcontroller.go

p0lyn0mial · 2025-07-29T09:07:38Z

@wangke19 is there anything else we should consider before merging this pr ?

pkg/operator/nodekubeconfigcontroller/nodekubeconfigcontroller.go

wangke19 · 2025-07-30T09:01:07Z

@wangke19 is there anything else we should consider before merging this pr ?

lgtm

p0lyn0mial · 2025-07-30T09:01:37Z

pkg/operator/nodekubeconfigcontroller/nodekubeconfigcontroller.go

 	}
 	requiredSecret.Annotations[annotations.OpenShiftComponent] = "kube-apiserver"
+	// Copy not-before/not-after annotations from systemAdminClientCert
+if systemAdminCredsSecret.Annotations != nil {


in go reading from nil map works, no ?
xref: https://go.dev/play/p/hrmvBX3L2Q1

yes, you are right.
Reading from a nil map does NOT panic — it returns the zero value for the type.
Here’s an idiomatic and explicit version.

if val, ok := systemAdminCredsSecret.Annotations[certrotation.CertificateNotBeforeAnnotation]; ok && len(val) > 0 { requiredSecret.Annotations[certrotation.CertificateNotBeforeAnnotation] = val } if val, ok := systemAdminCredsSecret.Annotations[certrotation.CertificateNotAfterAnnotation]; ok && len(val) > 0 { requiredSecret.Annotations[certrotation.CertificateNotAfterAnnotation] = val }

if len(systemAdminCredsSecret.Annotations[certrotation.CertificateNotBeforeAnnotation]) > 0 also works, right ?

yes, the original code snippet is safe as-is regarding nil map reads. Suggestion improves readability and maintainability for other devs

I would prefer len(systemAdminCredsSecret.Annotations[certrotation.CertificateNotBeforeAnnotation]) > 0 as it is idiomatic and more concise. maybe it is just me. do you mind brining the original version?

Copy not-before and not-after annotations from the system:admin secret

p0lyn0mial · 2025-07-30T10:35:29Z

/lgtm

openshift-ci · 2025-07-30T10:35:50Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: p0lyn0mial, vrutkovs

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:

~~OWNERS~~ [p0lyn0mial,vrutkovs]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot · 2025-07-30T16:05:51Z

/retest-required

Remaining retests: 0 against base HEAD 7e89682 and 2 for PR HEAD 80afb73 in total

openshift-ci · 2025-07-30T18:37:40Z

@vrutkovs: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-aws-ovn-serial	`c4d8d5c`	link	true	`/test e2e-aws-ovn-serial`
ci/prow/e2e-gcp-operator-single-node	`80afb73`	link	false	`/test e2e-gcp-operator-single-node`
ci/prow/e2e-azure-ovn	`80afb73`	link	false	`/test e2e-azure-ovn`
ci/prow/e2e-aws-operator-disruptive-single-node	`80afb73`	link	false	`/test e2e-aws-operator-disruptive-single-node`

Full PR test history. Your PR dashboard.

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-sigs/prow repository. I understand the commands that are listed here.

openshift-ci-robot · 2025-07-30T18:40:04Z

@vrutkovs: Jira Issue OCPBUGS-44842: Some pull requests linked via external trackers have merged:

The following pull requests linked via external trackers have not merged:

openshift/cluster-authentication-operator#776 is open

These pull request must merge or be unlinked from the Jira bug in order for it to move to the next state. Once unlinked, request a bug refresh with /jira refresh.

Jira Issue OCPBUGS-44842 has not been moved to the MODIFIED state.

In response to this:

Pull in openshift/library-go#1971 to make sure most secrets created by controller would have refresh-period annotation set.

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.

openshift-bot · 2025-07-30T22:10:03Z

[ART PR BUILD NOTIFIER]

Distgit: ose-cluster-kube-apiserver-operator
This PR has been included in build ose-cluster-kube-apiserver-operator-container-v4.20.0-202507302114.p0.ga08323d.assembly.stream.el9.
All builds following this will include this PR.

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 24, 2025

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 24, 2025

openshift-ci-robot added the jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. label Jul 24, 2025

openshift-ci bot requested a review from wangke19 July 24, 2025 07:01

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 24, 2025

vrutkovs force-pushed the cert-annotations-not-before-not-after-v2 branch from 7a50d40 to a372921 Compare July 24, 2025 07:34

vrutkovs force-pushed the cert-annotations-not-before-not-after-v2 branch from a372921 to ea69044 Compare July 24, 2025 08:05

vrutkovs force-pushed the cert-annotations-not-before-not-after-v2 branch from ea69044 to fed52f9 Compare July 24, 2025 10:26

openshift-ci-robot mentioned this pull request Jul 24, 2025

OCPBUGS-57049: Update cert annotations openshift/library-go#1971

Merged

Bump library-go to lastest master

52131d4

vrutkovs force-pushed the cert-annotations-not-before-not-after-v2 branch from fed52f9 to c4d8d5c Compare July 24, 2025 12:57

vrutkovs changed the title ~~WIP OCPBUGS-57049: Set not-before/not-after annotations~~ OCPBUGS-57049: Set not-before/not-after annotations Jul 24, 2025

vrutkovs marked this pull request as ready for review July 24, 2025 12:58

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 24, 2025

openshift-ci bot requested review from benluddy and tkashem July 24, 2025 12:59

openshift-ci bot requested a review from p0lyn0mial July 24, 2025 12:59

vrutkovs changed the title ~~OCPBUGS-57049: Set not-before/not-after annotations~~ OCPBUGS-44842: Set not-before/not-after annotations Jul 24, 2025

openshift-ci-robot added jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. and removed jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. labels Jul 24, 2025

openshift-ci-robot added the jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. label Jul 24, 2025

vrutkovs force-pushed the cert-annotations-not-before-not-after-v2 branch 2 times, most recently from cf3ea12 to e7cd82e Compare July 28, 2025 11:55

p0lyn0mial reviewed Jul 28, 2025

View reviewed changes

pkg/operator/nodekubeconfigcontroller/nodekubeconfigcontroller_test.go Outdated Show resolved Hide resolved

pkg/operator/nodekubeconfigcontroller/nodekubeconfigcontroller_test.go Show resolved Hide resolved

nodekubeconfigcontroller: fix typo

f1f9a76

infrastuctureLister -> infrastructureLister

vrutkovs force-pushed the cert-annotations-not-before-not-after-v2 branch 2 times, most recently from b184b35 to 096e30f Compare July 28, 2025 15:27

p0lyn0mial reviewed Jul 29, 2025

View reviewed changes

pkg/operator/nodekubeconfigcontroller/nodekubeconfigcontroller.go Show resolved Hide resolved

wangke19 reviewed Jul 30, 2025

View reviewed changes

pkg/operator/nodekubeconfigcontroller/nodekubeconfigcontroller.go Show resolved Hide resolved

vrutkovs force-pushed the cert-annotations-not-before-not-after-v2 branch from e2db6a1 to bb4d6a9 Compare July 30, 2025 08:59

p0lyn0mial reviewed Jul 30, 2025

View reviewed changes

vrutkovs force-pushed the cert-annotations-not-before-not-after-v2 branch from bb4d6a9 to 87ea901 Compare July 30, 2025 09:33

nodekubeconfig: set not-before/not-after annotations

80afb73

Copy not-before and not-after annotations from the system:admin secret

vrutkovs force-pushed the cert-annotations-not-before-not-after-v2 branch from 87ea901 to 80afb73 Compare July 30, 2025 09:51

openshift-ci bot assigned p0lyn0mial Jul 30, 2025

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 30, 2025

openshift-merge-bot bot merged commit a08323d into openshift:main Jul 30, 2025
16 of 19 checks passed

This was referenced Aug 25, 2025

OCPBUGS-57049: certrotation: move test case name outside of AutoRegenerateAfterOfflineExpiry #1870

Merged

Revert "OCPBUGS-57049: certrotation: move test case name outside of AutoRegenerateAfterOfflineExpiry" #1907

Merged

openshift-ci-robot mentioned this pull request Sep 19, 2025

Re-apply "OCPBUGS-57049: certrotation: move test case name outside of AutoRegenerateAfterOfflineExpiry" #1910

Merged

OCPBUGS-44842: Set not-before/not-after annotations #1873

OCPBUGS-44842: Set not-before/not-after annotations #1873

Uh oh!

Conversation

vrutkovs commented Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented Jul 24, 2025

Uh oh!

openshift-ci-robot commented Jul 24, 2025

Uh oh!

vrutkovs commented Jul 24, 2025

Uh oh!

vrutkovs commented Jul 24, 2025

Uh oh!

vrutkovs commented Jul 24, 2025

Uh oh!

vrutkovs commented Jul 24, 2025

Uh oh!

vrutkovs commented Jul 24, 2025

Uh oh!

openshift-ci-robot commented Jul 24, 2025

Uh oh!

vrutkovs commented Jul 24, 2025

Uh oh!

wangke19 commented Jul 28, 2025

Uh oh!

vrutkovs commented Jul 28, 2025

Uh oh!

p0lyn0mial left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

wangke19 commented Jul 29, 2025

Uh oh!

p0lyn0mial left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

p0lyn0mial commented Jul 29, 2025

Uh oh!

Uh oh!

wangke19 commented Jul 30, 2025

Uh oh!

p0lyn0mial Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

wangke19 Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

p0lyn0mial Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

wangke19 Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

p0lyn0mial Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

vrutkovs Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

p0lyn0mial Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

p0lyn0mial commented Jul 30, 2025

Uh oh!

openshift-ci bot commented Jul 30, 2025

Uh oh!

openshift-ci-robot commented Jul 30, 2025

Uh oh!

openshift-ci bot commented Jul 30, 2025

Uh oh!

Uh oh!

openshift-ci-robot commented Jul 30, 2025

Uh oh!

openshift-bot commented Jul 30, 2025

Uh oh!

vrutkovs commented Jul 24, 2025 •

edited

Loading

wangke19 Jul 30, 2025 •

edited

Loading