Merge pull request #146 from pfnet/principal-integrity

Ensure principal and image pull secret integrity

Author: ordovicia

internal/controller/imagepullsecret.go

@@ -29,14 +29,15 @@ import (
 // buildImagePullSecret builds a Kubernetes Secret definition for an image pull secrets.
 // The built Secret will have
 // - a label to select them by the ServiceAccount name,
-// - an annotation to store the expiration time, and
+// - an annotation to store the principal and expiration time, and
 // - an owner reference to the ServiceAccount so that they will be deleted when the ServiceAccount no longer exists.
 func buildImagePullSecret(
 	serviceAccount *corev1.ServiceAccount,
 	secretName string,
 	registry string,
 	username string,
 	password string,
+	principal string,
 	expiresAt time.Time,
 ) (*corev1.Secret, error) {
 	type dockerConfigEntry struct {
@@ -70,6 +71,7 @@ func buildImagePullSecret(
 				labelKeyServiceAccount: serviceAccount.GetName(),
 			},
 			Annotations: map[string]string{
+				annotationKeyPrincipal: principal,
 				annotationKeyExpiresAt: expiresAt.Format(time.RFC3339),
 			},
 			OwnerReferences: []metav1.OwnerReference{

internal/controller/imagepullsecret_test.go

@@ -40,9 +40,10 @@ func TestBuildImagePullSecret(t *testing.T) {
 	registry := "asia-northeast1-docker.pkg.dev"
 	username := "oauth2accesstoken"
 	password := "0xc0bebeef"
+	principal := "[email protected]"
 	expiresAt := time.Now().Add(time.Hour)
 
-	actual, err := buildImagePullSecret(sa, "secret-0", registry, username, password, expiresAt)
+	actual, err := buildImagePullSecret(sa, "secret-0", registry, username, password, principal, expiresAt)
 	if err != nil {
 		t.Errorf("Failed to build an image pull secret: %v", err)
 	}
@@ -54,6 +55,7 @@ func TestBuildImagePullSecret(t *testing.T) {
 			"imagepullsecrets.preferred.jp/service-account": "serviceaccount-0",
 		},
 		Annotations: map[string]string{
+			"imagepullsecrets.preferred.jp/principal":  principal,
 			"imagepullsecrets.preferred.jp/expires-at": expiresAt.Format(time.RFC3339),
 		},
 		OwnerReferences: []metav1.OwnerReference{

internal/controller/metadata.go

@@ -33,6 +33,8 @@ const (
 
 	annotationKeySecretName = metadataKeyPrefix + "secret-name"
 
+	// Annotation for Secrets to store the principal used to provision the Secret.
+	annotationKeyPrincipal = metadataKeyPrefix + "principal"
 	// Annotation for Secrets to store the expiration time.
 	annotationKeyExpiresAt = metadataKeyPrefix + "expires-at"
 

internal/controller/serviceaccount_controller.go

@@ -157,7 +157,7 @@ func (r *serviceAccountReconciler) provisionImagePullSecretForPrincipal(
 ) (expiresAt time.Time, _ error) {
 	logger := log.FromContext(ctx).WithValues("secret", secretName, "principal", principal)
 
-	should, exp, err := r.shouldCreateOrRefreshImagePullSecret(ctx, logger, sa, secretName)
+	should, exp, err := r.shouldCreateOrRefreshImagePullSecret(ctx, logger, sa, secretName, principal)
 	if err != nil {
 		return time.Time{}, fmt.Errorf("failed to determine if an image pull secret should be created or refreshed: %w", err)
 	}
@@ -183,10 +183,10 @@ func (r *serviceAccountReconciler) provisionImagePullSecretForPrincipal(
 }
 
 func (r *serviceAccountReconciler) shouldCreateOrRefreshImagePullSecret(
-	ctx context.Context, logger logr.Logger, sa *corev1.ServiceAccount, name string,
+	ctx context.Context, logger logr.Logger, sa *corev1.ServiceAccount, secretName string, principal string,
 ) (should bool, expiresAt time.Time, _ error) {
 	// Check if the image pull secret exists.
-	secretKey := client.ObjectKey{Namespace: sa.GetNamespace(), Name: name}
+	secretKey := client.ObjectKey{Namespace: sa.GetNamespace(), Name: secretName}
 
 	secret := &corev1.Secret{}
 	if err := r.Get(ctx, secretKey, secret); err != nil {
@@ -198,6 +198,14 @@ func (r *serviceAccountReconciler) shouldCreateOrRefreshImagePullSecret(
 		return false, time.Time{}, fmt.Errorf("failed to check the existing of an image pull secret: %w", err)
 	}
 
+	// Check if the image pull secret was provisioned for the principal.
+	if secret.Annotations[annotationKeyPrincipal] != principal {
+		logger.Info(
+			"Image pull secret was provisioned for a different principal. Should be provisioned for the current one.",
+		)
+		return true, time.Time{}, nil
+	}
+
 	// Check if the image pull secret is attached to the ServiceAccount.
 	if !r.imagePullSecretAttached(sa, secret.GetName()) {
 		logger.Info("Image pull secret is not attached to the ServiceAccount. Should be attached.")
@@ -248,7 +256,7 @@ func (r *serviceAccountReconciler) createOrRefreshImagePullSecret(
 
 	// Ensure an image pull secret from the access token.
 	secret, err := buildImagePullSecret(
-		sa, name, sa.Annotations[annotationKeyRegistry], username, token, expiresAt,
+		sa, name, sa.Annotations[annotationKeyRegistry], username, token, principal, expiresAt,
 	)
 	if err != nil {
 		return nil, time.Time{}, fmt.Errorf("failed to build image pull secret definition: %w", err)