diff --git a/pkg/backend/openshiftcluster/create.go b/pkg/backend/openshiftcluster/create.go index 1f0a5b814..dedfcf35b 100644 --- a/pkg/backend/openshiftcluster/create.go +++ b/pkg/backend/openshiftcluster/create.go @@ -119,7 +119,7 @@ func (m *Manager) Create(ctx context.Context) error { return err } - pullSecret, err = pullsecret.SetRegistryProfiles(pullSecret, m.doc.OpenShiftCluster.Properties.RegistryProfiles...) + pullSecret, _, err = pullsecret.SetRegistryProfiles(pullSecret, m.doc.OpenShiftCluster.Properties.RegistryProfiles...) if err != nil { return err } diff --git a/pkg/install/fixpullsecret.go b/pkg/install/fixpullsecret.go new file mode 100644 index 000000000..ba2707abf --- /dev/null +++ b/pkg/install/fixpullsecret.go @@ -0,0 +1,40 @@ +package install + +// Copyright (c) Microsoft Corporation. +// Licensed under the Apache License 2.0. + +import ( + "context" + + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/util/retry" + + "github.com/Azure/ARO-RP/pkg/util/pullsecret" +) + +func (i *Installer) fixPullSecret(ctx context.Context) error { + // TODO: this function does not currently reapply a pull secret in + // development mode. + + return retry.RetryOnConflict(retry.DefaultRetry, func() error { + ps, err := i.kubernetescli.CoreV1().Secrets("openshift-config").Get("pull-secret", metav1.GetOptions{}) + if err != nil { + return err + } + + pullSecret, changed, err := pullsecret.SetRegistryProfiles(string(ps.Data[v1.DockerConfigJsonKey]), i.doc.OpenShiftCluster.Properties.RegistryProfiles...) + if err != nil { + return err + } + + if !changed { + return nil + } + + ps.Data[v1.DockerConfigJsonKey] = []byte(pullSecret) + + _, err = i.kubernetescli.CoreV1().Secrets("openshift-config").Update(ps) + return err + }) +} diff --git a/pkg/install/fixpullsecret_test.go b/pkg/install/fixpullsecret_test.go new file mode 100644 index 000000000..0295510ed --- /dev/null +++ b/pkg/install/fixpullsecret_test.go @@ -0,0 +1,115 @@ +package install + +// Copyright (c) Microsoft Corporation. +// Licensed under the Apache License 2.0. + +import ( + "context" + "testing" + + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/fake" + ktesting "k8s.io/client-go/testing" + + "github.com/Azure/ARO-RP/pkg/api" +) + +func TestFixPullSecret(t *testing.T) { + ctx := context.Background() + + for _, tt := range []struct { + name string + current []byte + rps []*api.RegistryProfile + want string + wantUpdated bool + }{ + { + name: "missing pull secret", + rps: []*api.RegistryProfile{ + { + Name: "arosvc.azurecr.io", + Username: "fred", + Password: "enter", + }, + }, + want: `{"auths":{"arosvc.azurecr.io":{"auth":"ZnJlZDplbnRlcg=="}}}`, + wantUpdated: true, + }, + { + name: "modified pull secret", + current: []byte(`{"auths":{"arosvc.azurecr.io":{"auth":""}}}`), + rps: []*api.RegistryProfile{ + { + Name: "arosvc.azurecr.io", + Username: "fred", + Password: "enter", + }, + }, + want: `{"auths":{"arosvc.azurecr.io":{"auth":"ZnJlZDplbnRlcg=="}}}`, + wantUpdated: true, + }, + { + name: "no change", + current: []byte(`{"auths":{"arosvc.azurecr.io":{"auth":"ZnJlZDplbnRlcg=="}}}`), + rps: []*api.RegistryProfile{ + { + Name: "arosvc.azurecr.io", + Username: "fred", + Password: "enter", + }, + }, + want: `{"auths":{"arosvc.azurecr.io":{"auth":"ZnJlZDplbnRlcg=="}}}`, + }, + } { + t.Run(tt.name, func(t *testing.T) { + var updated bool + + fakecli := fake.NewSimpleClientset(&v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pull-secret", + Namespace: "openshift-config", + }, + Data: map[string][]byte{ + v1.DockerConfigJsonKey: tt.current, + }, + }) + + fakecli.PrependReactor("update", "secrets", func(action ktesting.Action) (handled bool, ret runtime.Object, err error) { + updated = true + return false, nil, nil + }) + + i := &Installer{ + kubernetescli: fakecli, + doc: &api.OpenShiftClusterDocument{ + OpenShiftCluster: &api.OpenShiftCluster{ + Properties: api.OpenShiftClusterProperties{ + RegistryProfiles: tt.rps, + }, + }, + }, + } + + err := i.fixPullSecret(ctx) + if err != nil { + t.Error(err) + } + + if updated != tt.wantUpdated { + t.Fatal(updated) + } + + s, err := i.kubernetescli.CoreV1().Secrets("openshift-config").Get("pull-secret", metav1.GetOptions{}) + if err != nil { + t.Error(err) + } + + if string(s.Data[v1.DockerConfigJsonKey]) != tt.want { + t.Error(string(s.Data[v1.DockerConfigJsonKey])) + } + }) + } +} diff --git a/pkg/install/install.go b/pkg/install/install.go index 6f5234e77..32c24bdaa 100644 --- a/pkg/install/install.go +++ b/pkg/install/install.go @@ -150,6 +150,7 @@ func (i *Installer) AdminUpgrade(ctx context.Context) error { action(i.initializeKubernetesClients), action(i.ensureBillingRecord), // belt and braces action(i.fixLBProbes), + action(i.fixPullSecret), action(i.ensureGenevaLogging), action(i.upgradeCluster), diff --git a/pkg/util/pullsecret/pullsecret.go b/pkg/util/pullsecret/pullsecret.go index c3b244ab5..6335b4d70 100644 --- a/pkg/util/pullsecret/pullsecret.go +++ b/pkg/util/pullsecret/pullsecret.go @@ -6,6 +6,7 @@ package pullsecret import ( "encoding/base64" "encoding/json" + "reflect" "github.com/Azure/ARO-RP/pkg/api" ) @@ -14,7 +15,7 @@ type pullSecret struct { Auths map[string]map[string]interface{} `json:"auths,omitempty"` } -func SetRegistryProfiles(_ps string, rps ...*api.RegistryProfile) (string, error) { +func SetRegistryProfiles(_ps string, rps ...*api.RegistryProfile) (string, bool, error) { if _ps == "" { _ps = "{}" } @@ -23,21 +24,29 @@ func SetRegistryProfiles(_ps string, rps ...*api.RegistryProfile) (string, error err := json.Unmarshal([]byte(_ps), &ps) if err != nil { - return "", err + return "", false, err } if ps.Auths == nil { ps.Auths = map[string]map[string]interface{}{} } + var changed bool + for _, rp := range rps { - ps.Auths[rp.Name] = map[string]interface{}{ + v := map[string]interface{}{ "auth": base64.StdEncoding.EncodeToString([]byte(rp.Username + ":" + string(rp.Password))), } + + if !reflect.DeepEqual(ps.Auths[rp.Name], v) { + changed = true + } + + ps.Auths[rp.Name] = v } b, err := json.Marshal(ps) - return string(b), err + return string(b), changed, err } // Merge returns _ps over _base. If both _ps and _base have a given key, the diff --git a/pkg/util/pullsecret/pullsecret_test.go b/pkg/util/pullsecret/pullsecret_test.go index bf5060146..d830fe39b 100644 --- a/pkg/util/pullsecret/pullsecret_test.go +++ b/pkg/util/pullsecret/pullsecret_test.go @@ -15,18 +15,26 @@ func TestSetRegistryProfiles(t *testing.T) { original := `{"auths":{"arosvc.azurecr.io":{"auth":"x"},"registry.redhat.io":{"auth":"y"}}}` for _, tt := range []struct { - name string - ps string - rp *api.RegistryProfile - want *pullSecret + name string + ps string + rps []*api.RegistryProfile + want *pullSecret + wantChanged bool }{ { - name: "replace", + name: "add and replace", ps: original, - rp: &api.RegistryProfile{ - Name: "arosvc.azurecr.io", - Username: "fred", - Password: "enter", + rps: []*api.RegistryProfile{ + { + Name: "arosvc.azurecr.io", + Username: "fred", + Password: "enter", + }, + { + Name: "arosvc-int.azurecr.io", + Username: "fred", + Password: "enter", + }, }, want: &pullSecret{ Auths: map[string]map[string]interface{}{ @@ -36,21 +44,50 @@ func TestSetRegistryProfiles(t *testing.T) { "registry.redhat.io": { "auth": "y", }, + "arosvc-int.azurecr.io": { + "auth": "ZnJlZDplbnRlcg==", + }, }, }, + wantChanged: true, }, { - name: "add", - ps: original, - rp: &api.RegistryProfile{ - Name: "arosvc-int.azurecr.io", - Username: "fred", - Password: "enter", + name: "new", + rps: []*api.RegistryProfile{ + { + Name: "arosvc.azurecr.io", + Username: "fred", + Password: "enter", + }, }, want: &pullSecret{ Auths: map[string]map[string]interface{}{ "arosvc.azurecr.io": { - "auth": "x", + "auth": "ZnJlZDplbnRlcg==", + }, + }, + }, + wantChanged: true, + }, + { + name: "no change", + ps: `{"auths":{"arosvc.azurecr.io":{"auth":"ZnJlZDplbnRlcg=="},"arosvc-int.azurecr.io":{"auth":"ZnJlZDplbnRlcg=="},"registry.redhat.io":{"auth":"y"}}}`, + rps: []*api.RegistryProfile{ + { + Name: "arosvc.azurecr.io", + Username: "fred", + Password: "enter", + }, + { + Name: "arosvc-int.azurecr.io", + Username: "fred", + Password: "enter", + }, + }, + want: &pullSecret{ + Auths: map[string]map[string]interface{}{ + "arosvc.azurecr.io": { + "auth": "ZnJlZDplbnRlcg==", }, "registry.redhat.io": { "auth": "y", @@ -61,28 +98,17 @@ func TestSetRegistryProfiles(t *testing.T) { }, }, }, - { - name: "new", - rp: &api.RegistryProfile{ - Name: "arosvc.azurecr.io", - Username: "fred", - Password: "enter", - }, - want: &pullSecret{ - Auths: map[string]map[string]interface{}{ - "arosvc.azurecr.io": { - "auth": "ZnJlZDplbnRlcg==", - }, - }, - }, - }, } { t.Run(tt.name, func(t *testing.T) { - ps, err := SetRegistryProfiles(tt.ps, tt.rp) + ps, changed, err := SetRegistryProfiles(tt.ps, tt.rps...) if err != nil { t.Fatal(err) } + if changed != tt.wantChanged { + t.Error(changed) + } + var got *pullSecret err = json.Unmarshal([]byte(ps), &got) if err != nil {