Improve ServiceAccount Signing Key Rotation Procedure (#7313)

* Add `lastInitiationFinishedTime` and `lastCompletionTriggeredTime` to API So far, it was not possible to get to know when the initiation has been finished nor when the completion has been triggered. This was only done for those rotation structs which feature a 'Phase' field (for the other's it's useless, hence not added). * [make generate] * Make use of new fields * Drop assumptions about "old service account secrets" Earlier, the code implicitly assumed that all secrets except the first on in the `ServiceAccount.secrets[]` list are old. However, this is not true when the user interferes while the rotation is running. This commit improves the code to REALLY check whether the secret is old by comparing its creation timestamp with the timestamp when the last rotation initiation has finished. All secrets created after this time have definitely been signed with the new key, all others not or not necessarily. * Address PR review feedback
gardener · Jan 13, 2023 · 1c2fe03 · 1c2fe03
1 parent b6d3850
commit 1c2fe03
Show file tree

Hide file tree

Showing 28 changed files with 2,688 additions and 1,686 deletions.
diff --git a/charts/gardener/operator/templates/customresouredefintion.yaml b/charts/gardener/operator/templates/customresouredefintion.yaml
@@ -295,6 +295,18 @@ spec:
                               successfully completed.
                             format: date-time
                             type: string
+                          lastCompletionTriggeredTime:
+                            description: LastCompletionTriggeredTime is the recent
+                              time when the certificate authority credential rotation
+                              completion was triggered.
+                            format: date-time
+                            type: string
+                          lastInitiationFinishedTime:
+                            description: LastInitiationFinishedTime is the recent
+                              time when the certificate authority credential rotation
+                              initiation was completed.
+                            format: date-time
+                            type: string
                           lastInitiationTime:
                             description: LastInitiationTime is the most recent time
                               when the certificate authority credential rotation was

diff --git a/docs/api-reference/core.md b/docs/api-reference/core.md
@@ -2315,6 +2315,21 @@ CredentialsRotationPhase
 </tr>
 <tr>
 <td>
+<code>lastCompletionTime</code></br>
+<em>
+<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#time-v1-meta">
+Kubernetes meta/v1.Time
+</a>
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>LastCompletionTime is the most recent time when the certificate authority credential rotation was successfully
+completed.</p>
+</td>
+</tr>
+<tr>
+<td>
 <code>lastInitiationTime</code></br>
 <em>
 <a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#time-v1-meta">
@@ -2329,7 +2344,7 @@ Kubernetes meta/v1.Time
 </tr>
 <tr>
 <td>
-<code>lastCompletionTime</code></br>
+<code>lastInitiationFinishedTime</code></br>
 <em>
 <a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#time-v1-meta">
 Kubernetes meta/v1.Time
@@ -2338,10 +2353,25 @@ Kubernetes meta/v1.Time
 </td>
 <td>
 <em>(Optional)</em>
-<p>LastCompletionTime is the most recent time when the certificate authority credential rotation was successfully
+<p>LastInitiationFinishedTime is the recent time when the certificate authority credential rotation initiation was
 completed.</p>
 </td>
 </tr>
+<tr>
+<td>
+<code>lastCompletionTriggeredTime</code></br>
+<em>
+<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#time-v1-meta">
+Kubernetes meta/v1.Time
+</a>
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>LastCompletionTriggeredTime is the recent time when the certificate authority credential rotation completion was
+triggered.</p>
+</td>
+</tr>
 </tbody>
 </table>
 <h3 id="core.gardener.cloud/v1beta1.CRI">CRI
@@ -9437,6 +9467,21 @@ CredentialsRotationPhase
 </tr>
 <tr>
 <td>
+<code>lastCompletionTime</code></br>
+<em>
+<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#time-v1-meta">
+Kubernetes meta/v1.Time
+</a>
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>LastCompletionTime is the most recent time when the ETCD encryption key credential rotation was successfully
+completed.</p>
+</td>
+</tr>
+<tr>
+<td>
 <code>lastInitiationTime</code></br>
 <em>
 <a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#time-v1-meta">
@@ -9451,7 +9496,7 @@ Kubernetes meta/v1.Time
 </tr>
 <tr>
 <td>
-<code>lastCompletionTime</code></br>
+<code>lastInitiationFinishedTime</code></br>
 <em>
 <a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#time-v1-meta">
 Kubernetes meta/v1.Time
@@ -9460,10 +9505,25 @@ Kubernetes meta/v1.Time
 </td>
 <td>
 <em>(Optional)</em>
-<p>LastCompletionTime is the most recent time when the ETCD encryption key credential rotation was successfully
+<p>LastInitiationFinishedTime is the recent time when the certificate authority credential rotation initiation was
 completed.</p>
 </td>
 </tr>
+<tr>
+<td>
+<code>lastCompletionTriggeredTime</code></br>
+<em>
+<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#time-v1-meta">
+Kubernetes meta/v1.Time
+</a>
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>LastCompletionTriggeredTime is the recent time when the certificate authority credential rotation completion was
+triggered.</p>
+</td>
+</tr>
 </tbody>
 </table>
 <h3 id="core.gardener.cloud/v1beta1.ShootKubeconfigRotation">ShootKubeconfigRotation
@@ -9749,6 +9809,21 @@ CredentialsRotationPhase
 </tr>
 <tr>
 <td>
+<code>lastCompletionTime</code></br>
+<em>
+<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#time-v1-meta">
+Kubernetes meta/v1.Time
+</a>
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>LastCompletionTime is the most recent time when the service account key credential rotation was successfully
+completed.</p>
+</td>
+</tr>
+<tr>
+<td>
 <code>lastInitiationTime</code></br>
 <em>
 <a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#time-v1-meta">
@@ -9763,7 +9838,7 @@ Kubernetes meta/v1.Time
 </tr>
 <tr>
 <td>
-<code>lastCompletionTime</code></br>
+<code>lastInitiationFinishedTime</code></br>
 <em>
 <a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#time-v1-meta">
 Kubernetes meta/v1.Time
@@ -9772,10 +9847,25 @@ Kubernetes meta/v1.Time
 </td>
 <td>
 <em>(Optional)</em>
-<p>LastCompletionTime is the most recent time when the service account key credential rotation was successfully
+<p>LastInitiationFinishedTime is the recent time when the certificate authority credential rotation initiation was
 completed.</p>
 </td>
 </tr>
+<tr>
+<td>
+<code>lastCompletionTriggeredTime</code></br>
+<em>
+<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#time-v1-meta">
+Kubernetes meta/v1.Time
+</a>
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>LastCompletionTriggeredTime is the recent time when the certificate authority credential rotation completion was
+triggered.</p>
+</td>
+</tr>
 </tbody>
 </table>
 <h3 id="core.gardener.cloud/v1beta1.ShootSpec">ShootSpec

diff --git a/example/operator/10-crd-operator.gardener.cloud_gardens.yaml b/example/operator/10-crd-operator.gardener.cloud_gardens.yaml
@@ -295,6 +295,18 @@ spec:
                               successfully completed.
                             format: date-time
                             type: string
+                          lastCompletionTriggeredTime:
+                            description: LastCompletionTriggeredTime is the recent
+                              time when the certificate authority credential rotation
+                              completion was triggered.
+                            format: date-time
+                            type: string
+                          lastInitiationFinishedTime:
+                            description: LastInitiationFinishedTime is the recent
+                              time when the certificate authority credential rotation
+                              initiation was completed.
+                            format: date-time
+                            type: string
                           lastInitiationTime:
                             description: LastInitiationTime is the most recent time
                               when the certificate authority credential rotation was

diff --git a/pkg/api/core/shoot/warnings.go b/pkg/api/core/shoot/warnings.go
@@ -111,13 +111,13 @@ func getWarningsForIncompleteCredentialsRotation(shoot *core.Shoot, credentialsR
 
 	// Only consider credentials for which completion must be triggered explicitly by the user. Credentials which are
 	// rotated in "one phase" are excluded.
-	if rotation.CertificateAuthorities != nil && completionDue(rotation.CertificateAuthorities.LastInitiationTime, rotation.CertificateAuthorities.LastCompletionTime, recommendedCompletionInterval) {
+	if rotation.CertificateAuthorities != nil && completionDue(rotation.CertificateAuthorities.LastInitiationFinishedTime, rotation.CertificateAuthorities.LastCompletionTriggeredTime, recommendedCompletionInterval) {
 		warnings = append(warnings, completionWarning("certificate authorities", recommendedCompletionInterval))
 	}
-	if rotation.ETCDEncryptionKey != nil && completionDue(rotation.ETCDEncryptionKey.LastInitiationTime, rotation.ETCDEncryptionKey.LastCompletionTime, recommendedCompletionInterval) {
+	if rotation.ETCDEncryptionKey != nil && completionDue(rotation.ETCDEncryptionKey.LastInitiationFinishedTime, rotation.ETCDEncryptionKey.LastCompletionTriggeredTime, recommendedCompletionInterval) {
 		warnings = append(warnings, completionWarning("ETCD encryption key", recommendedCompletionInterval))
 	}
-	if rotation.ServiceAccountKey != nil && completionDue(rotation.ServiceAccountKey.LastInitiationTime, rotation.ServiceAccountKey.LastCompletionTime, recommendedCompletionInterval) {
+	if rotation.ServiceAccountKey != nil && completionDue(rotation.ServiceAccountKey.LastInitiationFinishedTime, rotation.ServiceAccountKey.LastCompletionTriggeredTime, recommendedCompletionInterval) {
 		warnings = append(warnings, completionWarning("ServiceAccount token signing key", recommendedCompletionInterval))
 	}
 
@@ -128,22 +128,22 @@ func initiationDue(lastInitiationTime *metav1.Time, threshold time.Duration) boo
 	return lastInitiationTime == nil || isOldEnough(lastInitiationTime.Time, threshold)
 }
 
-func completionDue(lastInitiationTime, lastCompletionTime *metav1.Time, threshold time.Duration) bool {
-	if lastInitiationTime == nil {
+func completionDue(lastInitiationFinishedTime, lastCompletionTriggeredTime *metav1.Time, threshold time.Duration) bool {
+	if lastInitiationFinishedTime == nil {
 		return false
 	}
-	if lastCompletionTime != nil && lastCompletionTime.Time.UTC().After(lastInitiationTime.Time.UTC()) {
+	if lastCompletionTriggeredTime != nil && lastCompletionTriggeredTime.Time.UTC().After(lastInitiationFinishedTime.Time.UTC()) {
 		return false
 	}
-	return isOldEnough(lastInitiationTime.Time, threshold)
+	return isOldEnough(lastInitiationFinishedTime.Time, threshold)
 }
 
 func isOldEnough(t time.Time, threshold time.Duration) bool {
 	return t.UTC().Add(threshold).Before(time.Now().UTC())
 }
 
 func completionWarning(credentials string, recommendedCompletionInterval time.Duration) string {
-	return fmt.Sprintf("the %s rotation was initiated more than %s ago and should be completed", credentials, recommendedCompletionInterval)
+	return fmt.Sprintf("the %s rotation initiation was finished more than %s ago and should be completed", credentials, recommendedCompletionInterval)
 }
 
 func getWarningsForPSPAdmissionPlugin(shoot *core.Shoot) string {

diff --git a/pkg/api/core/shoot/warnings_test.go b/pkg/api/core/shoot/warnings_test.go
@@ -114,22 +114,22 @@ var _ = Describe("Warnings", func() {
 						rotation.CertificateAuthorities.LastInitiationTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval / 2)}
 					},
 				),
-				Entry("ca completion is due (never completed yet)", ContainElement(ContainSubstring("the certificate authorities rotation was initiated more than")), nil,
+				Entry("ca completion is due (never completed yet)", ContainElement(ContainSubstring("the certificate authorities rotation initiation was finished more than")), nil,
 					func(rotation *core.ShootCredentialsRotation) {
-						rotation.CertificateAuthorities.LastInitiationTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval / 2)}
-						rotation.CertificateAuthorities.LastCompletionTime = nil
+						rotation.CertificateAuthorities.LastInitiationFinishedTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval / 2)}
+						rotation.CertificateAuthorities.LastCompletionTriggeredTime = nil
 					},
 				),
-				Entry("ca completion is due (current rotation not completed)", ContainElement(ContainSubstring("the certificate authorities rotation was initiated more than")), nil,
+				Entry("ca completion is due (current rotation not completed)", ContainElement(ContainSubstring("the certificate authorities rotation initiation was finished more than")), nil,
 					func(rotation *core.ShootCredentialsRotation) {
-						rotation.CertificateAuthorities.LastInitiationTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval / 2)}
-						rotation.CertificateAuthorities.LastCompletionTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval)}
+						rotation.CertificateAuthorities.LastInitiationFinishedTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval / 2)}
+						rotation.CertificateAuthorities.LastCompletionTriggeredTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval)}
 					},
 				),
-				Entry("ca completion is not due (current rotation not completed)", Not(ContainElement(ContainSubstring("the certificate authorities rotation was initiated more than"))), nil,
+				Entry("ca completion is not due (current rotation not completed)", Not(ContainElement(ContainSubstring("the certificate authorities rotation initiation was finished more than"))), nil,
 					func(rotation *core.ShootCredentialsRotation) {
-						rotation.CertificateAuthorities.LastInitiationTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval / 2)}
-						rotation.CertificateAuthorities.LastCompletionTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval / 3)}
+						rotation.CertificateAuthorities.LastInitiationFinishedTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval / 2)}
+						rotation.CertificateAuthorities.LastCompletionTriggeredTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval / 3)}
 					},
 				),
 
@@ -148,22 +148,22 @@ var _ = Describe("Warnings", func() {
 						rotation.ETCDEncryptionKey.LastInitiationTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval / 2)}
 					},
 				),
-				Entry("etcdEncryptionKey completion is due (never completed yet)", ContainElement(ContainSubstring("the ETCD encryption key rotation was initiated more than")), nil,
+				Entry("etcdEncryptionKey completion is due (never completed yet)", ContainElement(ContainSubstring("the ETCD encryption key rotation initiation was finished more than")), nil,
 					func(rotation *core.ShootCredentialsRotation) {
-						rotation.ETCDEncryptionKey.LastInitiationTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval / 2)}
-						rotation.ETCDEncryptionKey.LastCompletionTime = nil
+						rotation.ETCDEncryptionKey.LastInitiationFinishedTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval / 2)}
+						rotation.ETCDEncryptionKey.LastCompletionTriggeredTime = nil
 					},
 				),
-				Entry("etcdEncryptionKey completion is due (current rotation not completed)", ContainElement(ContainSubstring("the ETCD encryption key rotation was initiated more than")), nil,
+				Entry("etcdEncryptionKey completion is due (current rotation not completed)", ContainElement(ContainSubstring("the ETCD encryption key rotation initiation was finished more than")), nil,
 					func(rotation *core.ShootCredentialsRotation) {
-						rotation.ETCDEncryptionKey.LastInitiationTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval / 2)}
-						rotation.ETCDEncryptionKey.LastCompletionTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval)}
+						rotation.ETCDEncryptionKey.LastInitiationFinishedTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval / 2)}
+						rotation.ETCDEncryptionKey.LastCompletionTriggeredTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval)}
 					},
 				),
-				Entry("etcdEncryptionKey completion is not due (current rotation not completed)", Not(ContainElement(ContainSubstring("the ETCD encryption key rotation was initiated more than"))), nil,
+				Entry("etcdEncryptionKey completion is not due (current rotation not completed)", Not(ContainElement(ContainSubstring("the ETCD encryption key rotation initiation was finished more than"))), nil,
 					func(rotation *core.ShootCredentialsRotation) {
-						rotation.ETCDEncryptionKey.LastInitiationTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval / 2)}
-						rotation.ETCDEncryptionKey.LastCompletionTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval / 3)}
+						rotation.ETCDEncryptionKey.LastInitiationFinishedTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval / 2)}
+						rotation.ETCDEncryptionKey.LastCompletionTriggeredTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval / 3)}
 					},
 				),
 
@@ -237,22 +237,22 @@ var _ = Describe("Warnings", func() {
 						rotation.ServiceAccountKey.LastInitiationTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval / 2)}
 					},
 				),
-				Entry("serviceAccountKey completion is due (never completed yet)", ContainElement(ContainSubstring("the ServiceAccount token signing key rotation was initiated more than")), nil,
+				Entry("serviceAccountKey completion is due (never completed yet)", ContainElement(ContainSubstring("the ServiceAccount token signing key rotation initiation was finished more than")), nil,
 					func(rotation *core.ShootCredentialsRotation) {
-						rotation.ServiceAccountKey.LastInitiationTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval / 2)}
-						rotation.ServiceAccountKey.LastCompletionTime = nil
+						rotation.ServiceAccountKey.LastInitiationFinishedTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval / 2)}
+						rotation.ServiceAccountKey.LastCompletionTriggeredTime = nil
 					},
 				),
-				Entry("serviceAccountKey completion is due (current rotation not completed)", ContainElement(ContainSubstring("the ServiceAccount token signing key rotation was initiated more than")), nil,
+				Entry("serviceAccountKey completion is due (current rotation not completed)", ContainElement(ContainSubstring("the ServiceAccount token signing key rotation initiation was finished more than")), nil,
 					func(rotation *core.ShootCredentialsRotation) {
-						rotation.ServiceAccountKey.LastInitiationTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval / 2)}
-						rotation.ServiceAccountKey.LastCompletionTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval)}
+						rotation.ServiceAccountKey.LastInitiationFinishedTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval / 2)}
+						rotation.ServiceAccountKey.LastCompletionTriggeredTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval)}
 					},
 				),
-				Entry("serviceAccountKey completion is not due (current rotation not completed)", Not(ContainElement(ContainSubstring("the ServiceAccount token signing key rotation was initiated more than"))), nil,
+				Entry("serviceAccountKey completion is not due (current rotation not completed)", Not(ContainElement(ContainSubstring("the ServiceAccount token signing key rotation initiation was finished more than"))), nil,
 					func(rotation *core.ShootCredentialsRotation) {
-						rotation.ServiceAccountKey.LastInitiationTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval / 2)}
-						rotation.ServiceAccountKey.LastCompletionTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval / 3)}
+						rotation.ServiceAccountKey.LastInitiationFinishedTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval / 2)}
+						rotation.ServiceAccountKey.LastCompletionTriggeredTime = &metav1.Time{Time: time.Now().Add(-credentialsRotationInterval / 3)}
 					},
 				),