From b0d2d3b7a7cd52247ca8b88d7f032004819c3627 Mon Sep 17 00:00:00 2001 From: Marek Vacula Date: Tue, 21 Jul 2026 16:33:51 +0200 Subject: [PATCH 1/3] Refactor pull secret helper functions and update go-doc refactor existing pull-secret related helpers for reusability. Update documentation with details and upstream references. Update existing E2E test to use the new helper functions. https://redhat.atlassian.net/browse/ARO-27529 --- test/e2e/cluster_pullsecret.go | 40 ++------- test/util/framework/hcp_helper.go | 31 ------- test/util/framework/pullsecret_helper.go | 104 +++++++++++++++++++++++ test/util/framework/pullsecret_types.go | 29 ------- test/util/verifiers/pullsecret.go | 18 +++- 5 files changed, 130 insertions(+), 92 deletions(-) create mode 100644 test/util/framework/pullsecret_helper.go delete mode 100644 test/util/framework/pullsecret_types.go diff --git a/test/e2e/cluster_pullsecret.go b/test/e2e/cluster_pullsecret.go index f8be217574e..35c1526221e 100644 --- a/test/e2e/cluster_pullsecret.go +++ b/test/e2e/cluster_pullsecret.go @@ -16,7 +16,6 @@ package e2e import ( "context" - "encoding/base64" "encoding/json" "errors" "fmt" @@ -45,7 +44,7 @@ var _ = Describe("Customer", func() { // Tests the HyperShift HCCO global pull secret reconciliation flow: // additional-pull-secret in kube-system -> HCCO merges into global-pull-secret -> DaemonSet syncs to nodes - // Upstream documentation: https://hypershift.pages.dev/how-to/aws/global-pull-secret/ + // See https://hypershift.pages.dev/how-to/aws/global-pull-secret/ It("should be able to create an HCP cluster and manage pull secrets", labels.RequireNothing, labels.Critical, @@ -146,12 +145,9 @@ var _ = Describe("Customer", func() { Expect(err).NotTo(HaveOccurred(), "failed to create kubernetes client") By("creating test pull secret") - username := "test-user" - auth := base64.StdEncoding.EncodeToString([]byte(username + ":" + testPullSecretPassword)) - - testPullSecret, err := framework.CreateTestDockerConfigSecret( + testPullSecret, testRegistryAuth, err := framework.CreateTestDockerConfigSecret( testPullSecretHost, - username, + "test-user", testPullSecretPassword, testPullSecretEmail, pullSecretName, @@ -180,8 +176,8 @@ var _ = Describe("Customer", func() { "global-pull-secret", pullSecretNamespace, testPullSecretHost, - auth, - testPullSecretEmail, + testRegistryAuth.Auth, + testRegistryAuth.Email, ).Verify(ctx, adminRESTConfig) Expect(err).NotTo(HaveOccurred(), "failed to verify pull secret auth data for host.example.com in global-pull-secret") @@ -198,31 +194,13 @@ var _ = Describe("Customer", func() { redhatRegistryAuth, ok := pullSecretConfig.Auths[redhatRegistryHost] Expect(ok).To(BeTrue(), "registry.redhat.io credentials not found in pull-secret file") - redhatRegistryAuthString := redhatRegistryAuth.Auth - redhatRegistryEmail := redhatRegistryAuth.Email - By("updating additional-pull-secret to add registry.redhat.io credentials") - // Get the current additional-pull-secret currentSecret, err := kubeClient.CoreV1().Secrets(pullSecretNamespace).Get(ctx, pullSecretName, metav1.GetOptions{}) Expect(err).NotTo(HaveOccurred(), "failed to get existing additional-pull-secret") - // Parse the current dockerconfigjson - var currentConfig framework.DockerConfigJSON - err = json.Unmarshal(currentSecret.Data[corev1.DockerConfigJsonKey], ¤tConfig) - Expect(err).NotTo(HaveOccurred(), "failed to parse current pull secret") - - // Add registry.redhat.io credentials to the existing auths - currentConfig.Auths[redhatRegistryHost] = framework.RegistryAuth{ - Auth: redhatRegistryAuthString, - Email: redhatRegistryEmail, - } - - // Marshal back to JSON - updatedDockerConfigJSON, err := json.Marshal(currentConfig) - Expect(err).NotTo(HaveOccurred(), "failed to marshal updated docker config JSON with registry.redhat.io credentials") + err = framework.AddRegistryAuthToSecret(currentSecret, redhatRegistryHost, redhatRegistryAuth) + Expect(err).NotTo(HaveOccurred(), "failed to add registry.redhat.io credentials to additional-pull-secret") - // Update the secret - currentSecret.Data[corev1.DockerConfigJsonKey] = updatedDockerConfigJSON _, err = kubeClient.CoreV1().Secrets(pullSecretNamespace).Update(ctx, currentSecret, metav1.UpdateOptions{}) Expect(err).NotTo(HaveOccurred(), "failed to update additional-pull-secret with registry.redhat.io credentials") @@ -244,8 +222,8 @@ var _ = Describe("Customer", func() { "global-pull-secret", pullSecretNamespace, redhatRegistryHost, - redhatRegistryAuthString, - redhatRegistryEmail, + redhatRegistryAuth.Auth, + redhatRegistryAuth.Email, ).Verify(ctx, adminRESTConfig) Expect(err).NotTo(HaveOccurred(), "failed to verify registry.redhat.io auth data in global-pull-secret") diff --git a/test/util/framework/hcp_helper.go b/test/util/framework/hcp_helper.go index b25eabf03a8..e8d3d95d61e 100644 --- a/test/util/framework/hcp_helper.go +++ b/test/util/framework/hcp_helper.go @@ -20,7 +20,6 @@ import ( "crypto/rand" "crypto/rsa" "crypto/x509" - "encoding/base64" "encoding/json" "encoding/pem" "errors" @@ -162,36 +161,6 @@ func CreateClusterRoleBinding(ctx context.Context, subject string, adminRESTConf return nil } -// CreateTestDockerConfigSecret creates a Docker config secret for testing pull secret functionality -func CreateTestDockerConfigSecret(host, username, password, email, secretName, namespace string) (*corev1.Secret, error) { - auth := base64.StdEncoding.EncodeToString([]byte(username + ":" + password)) - - dockerConfig := DockerConfigJSON{ - Auths: map[string]RegistryAuth{ - host: { - Email: email, - Auth: auth, - }, - }, - } - - dockerConfigJSON, err := json.Marshal(dockerConfig) - if err != nil { - return nil, fmt.Errorf("failed to marshal docker config: %w", err) - } - - return &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: secretName, - Namespace: namespace, - }, - Type: corev1.SecretTypeDockerConfigJson, - Data: map[string][]byte{ - corev1.DockerConfigJsonKey: dockerConfigJSON, - }, - }, nil -} - // Helper to generate SSH key pair func GenerateSSHKeyPair() (publicKey string, privateKey string, err error) { // Generate RSA key pair diff --git a/test/util/framework/pullsecret_helper.go b/test/util/framework/pullsecret_helper.go new file mode 100644 index 00000000000..2cbd71e2558 --- /dev/null +++ b/test/util/framework/pullsecret_helper.go @@ -0,0 +1,104 @@ +// Copyright 2025 Microsoft Corporation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package framework + +import ( + "encoding/base64" + "encoding/json" + "fmt" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// RegistryAuth represents authentication credentials for a single container +// image registry. It models one entry inside the "auths" map of a +// kubernetes.io/dockerconfigjson Secret. The Auth field is a base64 encoding +// of "username:password"; Username and Email are optional metadata. +// +// See https://kubernetes.io/docs/concepts/configuration/secret/#docker-config-secrets +type RegistryAuth struct { + Username string `json:"username,omitempty"` + Email string `json:"email,omitempty"` + Auth string `json:"auth"` +} + +// DockerConfigJSON is the root structure stored under the .dockerconfigjson +// key of a kubernetes.io/dockerconfigjson Secret. Auths maps registry +// hostnames (e.g. "quay.io", "registry.redhat.io") to their credentials. +// +// See https://kubernetes.io/docs/concepts/configuration/secret/#docker-config-secrets +type DockerConfigJSON struct { + Auths map[string]RegistryAuth `json:"auths"` +} + +// CreateTestDockerConfigSecret builds a corev1.Secret of type +// kubernetes.io/dockerconfigjson containing credentials for a single registry. +// It returns both the Secret and the RegistryAuth it constructed, so callers +// can pass the auth data directly to verifiers without recomputing it. +// The returned Secret is suitable for use as the HCCO "additional-pull-secret" +// in kube-system, which HCCO merges into the cluster's global pull secret. +// +// See https://hypershift.pages.dev/how-to/aws/global-pull-secret/ +func CreateTestDockerConfigSecret(host, username, password, email, secretName, namespace string) (*corev1.Secret, RegistryAuth, error) { + registryAuth := RegistryAuth{ + Email: email, + Auth: base64.StdEncoding.EncodeToString([]byte(username + ":" + password)), + } + + dockerConfig := DockerConfigJSON{ + Auths: map[string]RegistryAuth{ + host: registryAuth, + }, + } + + dockerConfigJSON, err := json.Marshal(dockerConfig) + if err != nil { + return nil, RegistryAuth{}, fmt.Errorf("failed to marshal docker config: %w", err) + } + + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: namespace, + }, + Type: corev1.SecretTypeDockerConfigJson, + Data: map[string][]byte{ + corev1.DockerConfigJsonKey: dockerConfigJSON, + }, + }, registryAuth, nil +} + +// AddRegistryAuthToSecret adds or replaces a registry entry in an existing +// dockerconfigjson Secret. It unmarshals the Secret's current .dockerconfigjson +// data, inserts (or overwrites) the entry for host, and marshals the result +// back into the Secret's Data field. The caller is responsible for applying the +// updated Secret to the cluster (e.g. via a Kubernetes Update call). +func AddRegistryAuthToSecret(secret *corev1.Secret, host string, registryAuth RegistryAuth) error { + var config DockerConfigJSON + if err := json.Unmarshal(secret.Data[corev1.DockerConfigJsonKey], &config); err != nil { + return fmt.Errorf("failed to unmarshal pull secret: %w", err) + } + + config.Auths[host] = registryAuth + + updated, err := json.Marshal(config) + if err != nil { + return fmt.Errorf("failed to marshal updated docker config: %w", err) + } + + secret.Data[corev1.DockerConfigJsonKey] = updated + return nil +} diff --git a/test/util/framework/pullsecret_types.go b/test/util/framework/pullsecret_types.go deleted file mode 100644 index be71e8a7e11..00000000000 --- a/test/util/framework/pullsecret_types.go +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright 2025 Microsoft Corporation -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package framework - -// RegistryAuth represents authentication credentials for a single registry. -// This type models the structure of dockerconfigjson registry auth entries. -type RegistryAuth struct { - Username string `json:"username,omitempty"` - Email string `json:"email,omitempty"` - Auth string `json:"auth"` -} - -// DockerConfigJSON is the root structure for dockerconfigjson secret data. -// See: https://kubernetes.io/docs/concepts/configuration/secret/#docker-config-secrets -type DockerConfigJSON struct { - Auths map[string]RegistryAuth `json:"auths"` -} diff --git a/test/util/verifiers/pullsecret.go b/test/util/verifiers/pullsecret.go index 7191b31be33..45e9e8a9370 100644 --- a/test/util/verifiers/pullsecret.go +++ b/test/util/verifiers/pullsecret.go @@ -66,6 +66,13 @@ func (v verifyPullSecretMergedIntoGlobal) checkOnce(ctx context.Context, adminRE return nil } +// VerifyPullSecretMergedIntoGlobal polls until the kube-system/global-pull-secret +// on the data plane contains an auths entry for expectedHost, or the timeout +// expires. The global-pull-secret is created by HCCO's Global Pull Secret +// Controller when it detects an additional-pull-secret in kube-system and +// merges it with the original-pull-secret. +// +// See https://hypershift.pages.dev/how-to/aws/global-pull-secret/ func VerifyPullSecretMergedIntoGlobal(expectedHost string, timeout time.Duration) HostedClusterVerifier { return verifyPullSecretMergedIntoGlobal{ expectedHost: expectedHost, @@ -78,8 +85,12 @@ const ( globalPullSecretSyncerName = "global-pull-secret-syncer" ) -// VerifyGlobalPullSecretSyncer verifies the global-pull-secret-syncer DaemonSet in kube-system. +// VerifyGlobalPullSecretSyncer verifies the global-pull-secret-syncer +// DaemonSet in kube-system is ready. Until this DaemonSet is ready, nodes +// will not have the updated pull secret credentials. // It delegates to [VerifyDaemonSetReady]. +// +// See https://hypershift.pages.dev/how-to/aws/global-pull-secret/ func VerifyGlobalPullSecretSyncer(timeout time.Duration) HostedClusterVerifier { return VerifyDaemonSetReady(globalPullSecretSyncerNamespace, globalPullSecretSyncerName, timeout) } @@ -128,6 +139,11 @@ func (v verifyPullSecretAuthData) Verify(ctx context.Context, adminRESTConfig *r return nil } +// VerifyPullSecretAuthData performs a single-shot check that the named +// dockerconfigjson Secret contains the expected auth (base64-encoded +// "username:password") and email values for the given registry host. +// Call only after HCCO has finished merging (e.g. after +// [VerifyPullSecretMergedIntoGlobal] succeeds). func VerifyPullSecretAuthData(secretName, namespace, expectedHost, expectedAuth, expectedEmail string) HostedClusterVerifier { return verifyPullSecretAuthData{ secretName: secretName, From 0444566c06208bd13f418e5e0fbd73b16539354b Mon Sep 17 00:00:00 2001 From: Marek Vacula Date: Tue, 21 Jul 2026 16:46:12 +0200 Subject: [PATCH 2/3] add unit tests --- test/util/framework/pullsecret_helper_test.go | 111 ++++++++++++++++++ 1 file changed, 111 insertions(+) create mode 100644 test/util/framework/pullsecret_helper_test.go diff --git a/test/util/framework/pullsecret_helper_test.go b/test/util/framework/pullsecret_helper_test.go new file mode 100644 index 00000000000..7512e662d35 --- /dev/null +++ b/test/util/framework/pullsecret_helper_test.go @@ -0,0 +1,111 @@ +// Copyright 2026 Microsoft Corporation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package framework + +import ( + "encoding/base64" + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + corev1 "k8s.io/api/core/v1" +) + +func TestCreateTestDockerConfigSecret(t *testing.T) { + t.Parallel() + + secret, registryAuth, err := CreateTestDockerConfigSecret( + "registry.example.com", + "user", + "pass", + "user@example.com", + "my-secret", + "my-namespace", + ) + require.NoError(t, err) + + assert.Equal(t, "my-secret", secret.Name) + assert.Equal(t, "my-namespace", secret.Namespace) + assert.Equal(t, corev1.SecretTypeDockerConfigJson, secret.Type) + + expectedAuth := base64.StdEncoding.EncodeToString([]byte("user:pass")) + assert.Equal(t, expectedAuth, registryAuth.Auth) + assert.Equal(t, "user@example.com", registryAuth.Email) + + var config DockerConfigJSON + require.NoError(t, json.Unmarshal(secret.Data[corev1.DockerConfigJsonKey], &config)) + + hostAuth, exists := config.Auths["registry.example.com"] + assert.True(t, exists, "expected registry.example.com in auths") + assert.Equal(t, registryAuth, hostAuth, "returned RegistryAuth must match what is in the Secret") +} + +func TestAddRegistryAuthToSecret(t *testing.T) { + t.Parallel() + + secret, originalAuth, err := CreateTestDockerConfigSecret( + "original.example.com", + "user1", + "pass1", + "user1@example.com", + "test-secret", + "default", + ) + require.NoError(t, err) + + newAuth := RegistryAuth{ + Auth: base64.StdEncoding.EncodeToString([]byte("user2:pass2")), + Email: "user2@example.com", + } + err = AddRegistryAuthToSecret(secret, "new.example.com", newAuth) + require.NoError(t, err) + + var config DockerConfigJSON + require.NoError(t, json.Unmarshal(secret.Data[corev1.DockerConfigJsonKey], &config)) + + assert.Contains(t, config.Auths, "original.example.com", "original entry must be preserved") + assert.Equal(t, originalAuth, config.Auths["original.example.com"]) + assert.Contains(t, config.Auths, "new.example.com", "new entry must be present") + assert.Equal(t, newAuth, config.Auths["new.example.com"]) +} + +func TestAddRegistryAuthToSecret_OverwritesExisting(t *testing.T) { + t.Parallel() + + secret, _, err := CreateTestDockerConfigSecret( + "registry.example.com", + "user", + "pass", + "old@example.com", + "test-secret", + "default", + ) + require.NoError(t, err) + + updatedAuth := RegistryAuth{ + Auth: base64.StdEncoding.EncodeToString([]byte("newuser:newpass")), + Email: "new@example.com", + } + err = AddRegistryAuthToSecret(secret, "registry.example.com", updatedAuth) + require.NoError(t, err) + + var config DockerConfigJSON + require.NoError(t, json.Unmarshal(secret.Data[corev1.DockerConfigJsonKey], &config)) + + assert.Len(t, config.Auths, 1) + assert.Equal(t, updatedAuth, config.Auths["registry.example.com"]) +} From 4c5b0b7eff1e0fe420fa2b9ec0aa253670382050 Mon Sep 17 00:00:00 2001 From: Marek Vacula Date: Mon, 27 Jul 2026 09:33:11 +0200 Subject: [PATCH 3/3] handle nil Auth, add unit test --- test/util/framework/pullsecret_helper.go | 3 +++ test/util/framework/pullsecret_helper_test.go | 23 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/test/util/framework/pullsecret_helper.go b/test/util/framework/pullsecret_helper.go index 2cbd71e2558..06346d5d497 100644 --- a/test/util/framework/pullsecret_helper.go +++ b/test/util/framework/pullsecret_helper.go @@ -92,6 +92,9 @@ func AddRegistryAuthToSecret(secret *corev1.Secret, host string, registryAuth Re return fmt.Errorf("failed to unmarshal pull secret: %w", err) } + if config.Auths == nil { + config.Auths = make(map[string]RegistryAuth) + } config.Auths[host] = registryAuth updated, err := json.Marshal(config) diff --git a/test/util/framework/pullsecret_helper_test.go b/test/util/framework/pullsecret_helper_test.go index 7512e662d35..6f85c86aba5 100644 --- a/test/util/framework/pullsecret_helper_test.go +++ b/test/util/framework/pullsecret_helper_test.go @@ -109,3 +109,26 @@ func TestAddRegistryAuthToSecret_OverwritesExisting(t *testing.T) { assert.Len(t, config.Auths, 1) assert.Equal(t, updatedAuth, config.Auths["registry.example.com"]) } + +func TestAddRegistryAuthToSecret_NilAuth(t *testing.T) { + t.Parallel() + + secret := &corev1.Secret{ + Data: map[string][]byte{ + corev1.DockerConfigJsonKey: []byte(`{}`), + }, + } + + newAuth := RegistryAuth{ + Auth: base64.StdEncoding.EncodeToString([]byte("user:pass")), + Email: "user@example.com", + } + err := AddRegistryAuthToSecret(secret, "registry.example.com", newAuth) + require.NoError(t, err) + + var config DockerConfigJSON + require.NoError(t, json.Unmarshal(secret.Data[corev1.DockerConfigJsonKey], &config)) + + assert.Len(t, config.Auths, 1) + assert.Equal(t, newAuth, config.Auths["registry.example.com"]) +}