From d298fd38510a9282e96e0132ecdb69896bf72bcf Mon Sep 17 00:00:00 2001 From: "adam.orcholski" Date: Wed, 26 Jun 2024 18:00:53 +0200 Subject: [PATCH] All pull secrets added to OA,AG specs and used by ImageInstaller --- pkg/api/v1beta2/dynakube/properties.go | 16 +++--- pkg/controllers/csi/provisioner/controller.go | 9 ++- .../internal/statefulset/statefulset.go | 11 +++- .../internal/statefulset/statefulset_test.go | 3 +- pkg/controllers/dynakube/controller.go | 5 +- .../dynakube/controller_system_test.go | 6 +- pkg/controllers/dynakube/controller_test.go | 24 ++++---- .../dynakube/dtpullsecret/reconciler.go | 4 -- .../dynakube/oneagent/daemonset/daemonset.go | 9 ++- .../codemodule/installer/image/installer.go | 3 +- .../installer/image/installer_test.go | 7 ++- pkg/oci/dockerkeychain/docker_keychain.go | 55 +++++++++++++++++++ 12 files changed, 109 insertions(+), 43 deletions(-) diff --git a/pkg/api/v1beta2/dynakube/properties.go b/pkg/api/v1beta2/dynakube/properties.go index 93ee7d7c5d..954ed48ed9 100644 --- a/pkg/api/v1beta2/dynakube/properties.go +++ b/pkg/api/v1beta2/dynakube/properties.go @@ -211,14 +211,16 @@ func (dk *DynaKube) PullSecretName() string { return dk.Name + PullSecretSuffix } -// PullSecretWithoutData returns a secret which can be used to query the actual secrets data from the cluster. -func (dk *DynaKube) PullSecretWithoutData() corev1.Secret { - return corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: dk.PullSecretName(), - Namespace: dk.Namespace, - }, +// PullSecretsNames returns the names of the pull secrets to be used for immutable images. +func (dk *DynaKube) PullSecretsNames() []string { + names := []string{ + dk.Name + PullSecretSuffix, } + if dk.Spec.CustomPullSecret != "" { + names = append(names, dk.Spec.CustomPullSecret) + } + + return names } func (dk *DynaKube) NeedsReadOnlyOneAgents() bool { diff --git a/pkg/controllers/csi/provisioner/controller.go b/pkg/controllers/csi/provisioner/controller.go index 6e389f811d..98ab9de145 100644 --- a/pkg/controllers/csi/provisioner/controller.go +++ b/pkg/controllers/csi/provisioner/controller.go @@ -32,7 +32,6 @@ import ( "github.com/Dynatrace/dynatrace-operator/pkg/injection/codemodule/installer" "github.com/Dynatrace/dynatrace-operator/pkg/injection/codemodule/installer/image" "github.com/Dynatrace/dynatrace-operator/pkg/injection/codemodule/installer/url" - "github.com/Dynatrace/dynatrace-operator/pkg/oci/registry" "github.com/Dynatrace/dynatrace-operator/pkg/util/dtotel" "github.com/pkg/errors" "github.com/spf13/afero" @@ -61,9 +60,9 @@ type OneAgentProvisioner struct { dynatraceClientBuilder dynatraceclient.Builder urlInstallerBuilder urlInstallerBuilder imageInstallerBuilder imageInstallerBuilder - registryClientBuilder registry.ClientBuilder - opts dtcsi.CSIOptions - path metadata.PathResolver + // registryClientBuilder registry.ClientBuilder + opts dtcsi.CSIOptions + path metadata.PathResolver } // NewOneAgentProvisioner returns a new OneAgentProvisioner @@ -80,7 +79,7 @@ func NewOneAgentProvisioner(mgr manager.Manager, opts dtcsi.CSIOptions, db metad dynatraceClientBuilder: dynatraceclient.NewBuilder(mgr.GetAPIReader()), urlInstallerBuilder: url.NewUrlInstaller, imageInstallerBuilder: image.NewImageInstaller, - registryClientBuilder: registry.NewClient, + // registryClientBuilder: registry.NewClient, } } diff --git a/pkg/controllers/dynakube/activegate/internal/statefulset/statefulset.go b/pkg/controllers/dynakube/activegate/internal/statefulset/statefulset.go index e54805ec39..4f45bd11a7 100644 --- a/pkg/controllers/dynakube/activegate/internal/statefulset/statefulset.go +++ b/pkg/controllers/dynakube/activegate/internal/statefulset/statefulset.go @@ -118,6 +118,13 @@ func (statefulSetBuilder Builder) addUserAnnotations(sts *appsv1.StatefulSet) { } func (statefulSetBuilder Builder) addTemplateSpec(sts *appsv1.StatefulSet) { + imagePullSecrets := make([]corev1.LocalObjectReference, 0) + for _, pullSecretName := range statefulSetBuilder.dynakube.PullSecretsNames() { + imagePullSecrets = append(imagePullSecrets, corev1.LocalObjectReference{ + Name: pullSecretName, + }) + } + podSpec := corev1.PodSpec{ Containers: statefulSetBuilder.buildBaseContainer(), NodeSelector: statefulSetBuilder.capability.Properties().NodeSelector, @@ -129,9 +136,7 @@ func (statefulSetBuilder Builder) addTemplateSpec(sts *appsv1.StatefulSet) { Type: corev1.SeccompProfileTypeRuntimeDefault, }, }, - ImagePullSecrets: []corev1.LocalObjectReference{ - {Name: statefulSetBuilder.dynakube.PullSecretName()}, - }, + ImagePullSecrets: imagePullSecrets, PriorityClassName: statefulSetBuilder.dynakube.Spec.ActiveGate.PriorityClassName, DNSPolicy: statefulSetBuilder.dynakube.Spec.ActiveGate.DNSPolicy, diff --git a/pkg/controllers/dynakube/activegate/internal/statefulset/statefulset_test.go b/pkg/controllers/dynakube/activegate/internal/statefulset/statefulset_test.go index 7cedc329d1..31e4313040 100644 --- a/pkg/controllers/dynakube/activegate/internal/statefulset/statefulset_test.go +++ b/pkg/controllers/dynakube/activegate/internal/statefulset/statefulset_test.go @@ -189,7 +189,8 @@ func TestAddTemplateSpec(t *testing.T) { assert.NotEmpty(t, spec.Containers) assert.NotEmpty(t, spec.Affinity) - assert.Equal(t, dynakube.PullSecretName(), spec.ImagePullSecrets[0].Name) + assert.Equal(t, len(dynakube.PullSecretsNames()), len(spec.ImagePullSecrets)) + assert.Equal(t, dynakube.PullSecretsNames()[0], spec.ImagePullSecrets[0].Name) }) t.Run("adds capability specific stuff", func(t *testing.T) { diff --git a/pkg/controllers/dynakube/controller.go b/pkg/controllers/dynakube/controller.go index 3ace6959c5..fdfdb99753 100644 --- a/pkg/controllers/dynakube/controller.go +++ b/pkg/controllers/dynakube/controller.go @@ -21,7 +21,6 @@ import ( "github.com/Dynatrace/dynatrace-operator/pkg/controllers/dynakube/proxy" "github.com/Dynatrace/dynatrace-operator/pkg/controllers/dynakube/token" "github.com/Dynatrace/dynatrace-operator/pkg/injection/namespace/mapper" - "github.com/Dynatrace/dynatrace-operator/pkg/oci/registry" "github.com/Dynatrace/dynatrace-operator/pkg/util/hasher" "github.com/Dynatrace/dynatrace-operator/pkg/util/kubeobjects/env" "github.com/Dynatrace/dynatrace-operator/pkg/util/kubesystem" @@ -67,7 +66,7 @@ func NewDynaKubeController(kubeClient client.Client, apiReader client.Reader, co clusterID: clusterID, dynatraceClientBuilder: dynatraceclient.NewBuilder(apiReader), istioClientBuilder: istio.NewClient, - registryClientBuilder: registry.NewClient, + // registryClientBuilder: registry.NewClient, deploymentMetadataReconcilerBuilder: deploymentmetadata.NewReconciler, activeGateReconcilerBuilder: activegate.NewReconciler, @@ -99,7 +98,7 @@ type Controller struct { dynatraceClientBuilder dynatraceclient.Builder config *rest.Config istioClientBuilder istio.ClientBuilder - registryClientBuilder registry.ClientBuilder + // registry.ClientBuilder deploymentMetadataReconcilerBuilder deploymentmetadata.ReconcilerBuilder activeGateReconcilerBuilder activegate.ReconcilerBuilder diff --git a/pkg/controllers/dynakube/controller_system_test.go b/pkg/controllers/dynakube/controller_system_test.go index 1555ae020c..3788d44a74 100644 --- a/pkg/controllers/dynakube/controller_system_test.go +++ b/pkg/controllers/dynakube/controller_system_test.go @@ -406,9 +406,9 @@ func createFakeClientAndReconciler(t *testing.T, mockClient dtclient.Client, ins mockDtcBuilder.On("BuildWithTokenVerification", mock.Anything).Return(mockClient, nil) controller := &Controller{ - client: fakeClient, - apiReader: fakeClient, - registryClientBuilder: createFakeRegistryClientBuilder(t), + client: fakeClient, + apiReader: fakeClient, + // registryClientBuilder: createFakeRegistryClientBuilder(t), dynatraceClientBuilder: mockDtcBuilder, fs: afero.Afero{Fs: afero.NewMemMapFs()}, deploymentMetadataReconcilerBuilder: deploymentmetadata.NewReconciler, diff --git a/pkg/controllers/dynakube/controller_test.go b/pkg/controllers/dynakube/controller_test.go index cdaa5b91f0..a6a315601e 100644 --- a/pkg/controllers/dynakube/controller_test.go +++ b/pkg/controllers/dynakube/controller_test.go @@ -19,15 +19,11 @@ import ( "github.com/Dynatrace/dynatrace-operator/pkg/controllers/dynakube/istio" "github.com/Dynatrace/dynatrace-operator/pkg/controllers/dynakube/oneagent" "github.com/Dynatrace/dynatrace-operator/pkg/controllers/dynakube/token" - "github.com/Dynatrace/dynatrace-operator/pkg/oci/registry" dtwebhook "github.com/Dynatrace/dynatrace-operator/pkg/webhook" dtclientmock "github.com/Dynatrace/dynatrace-operator/test/mocks/pkg/clients/dynatrace" controllermock "github.com/Dynatrace/dynatrace-operator/test/mocks/pkg/controllers" dtbuildermock "github.com/Dynatrace/dynatrace-operator/test/mocks/pkg/controllers/dynakube/dynatraceclient" injectionmock "github.com/Dynatrace/dynatrace-operator/test/mocks/pkg/controllers/dynakube/injection" - registrymock "github.com/Dynatrace/dynatrace-operator/test/mocks/pkg/oci/registry" - containerv1 "github.com/google/go-containerregistry/pkg/v1" - fakecontainer "github.com/google/go-containerregistry/pkg/v1/fake" "github.com/pkg/errors" "github.com/spf13/afero" "github.com/stretchr/testify/assert" @@ -343,10 +339,10 @@ func TestReconcileComponents(t *testing.T) { mockInjectionReconciler.On("Reconcile", mock.Anything).Return(errors.New("BOOM")) controller := &Controller{ - client: fakeClient, - apiReader: fakeClient, - fs: afero.Afero{Fs: afero.NewMemMapFs()}, - registryClientBuilder: createFakeRegistryClientBuilder(t), + client: fakeClient, + apiReader: fakeClient, + fs: afero.Afero{Fs: afero.NewMemMapFs()}, + // registryClientBuilder: createFakeRegistryClientBuilder(t), activeGateReconcilerBuilder: createActivegateReconcilerBuilder(mockActiveGateReconciler), injectionReconcilerBuilder: createInjectionReconcilerBuilder(mockInjectionReconciler), @@ -371,10 +367,10 @@ func TestReconcileComponents(t *testing.T) { mockInjectionReconciler.On("Reconcile", mock.Anything).Return(oaconnectioninfo.NoOneAgentCommunicationHostsError) controller := &Controller{ - client: fakeClient, - apiReader: fakeClient, - fs: afero.Afero{Fs: afero.NewMemMapFs()}, - registryClientBuilder: createFakeRegistryClientBuilder(t), + client: fakeClient, + apiReader: fakeClient, + fs: afero.Afero{Fs: afero.NewMemMapFs()}, + // registryClientBuilder: createFakeRegistryClientBuilder(t), activeGateReconcilerBuilder: createActivegateReconcilerBuilder(mockActiveGateReconciler), injectionReconcilerBuilder: createInjectionReconcilerBuilder(mockInjectionReconciler), } @@ -405,6 +401,7 @@ func createInjectionReconcilerBuilder(reconciler *injectionmock.Reconciler) inje } } +/* func createFakeRegistryClientBuilder(t *testing.T) func(options ...func(*registry.Client)) (registry.ImageGetter, error) { fakeRegistryClient := registrymock.NewImageGetter(t) fakeImage := &fakecontainer.FakeImage{} @@ -421,6 +418,7 @@ func createFakeRegistryClientBuilder(t *testing.T) func(options ...func(*registr return fakeRegistryClient, nil } } +*/ type errorClient struct { client.Client @@ -537,7 +535,7 @@ func TestTokenConditions(t *testing.T) { client: fakeClient, apiReader: fakeClient, dynatraceClientBuilder: mockDtcBuilder, - registryClientBuilder: createFakeRegistryClientBuilder(t), + // registryClientBuilder: createFakeRegistryClientBuilder(t), } _, err := controller.setupTokensAndClient(ctx, dynakube) diff --git a/pkg/controllers/dynakube/dtpullsecret/reconciler.go b/pkg/controllers/dynakube/dtpullsecret/reconciler.go index 0b7ace7f79..f8bba869db 100644 --- a/pkg/controllers/dynakube/dtpullsecret/reconciler.go +++ b/pkg/controllers/dynakube/dtpullsecret/reconciler.go @@ -39,10 +39,6 @@ func NewReconciler(clt client.Client, apiReader client.Reader, dynakube *dynatra } func (r *Reconciler) Reconcile(ctx context.Context) error { - if r.dynakube.Spec.CustomPullSecret != "" { - return nil - } - if !(r.dynakube.NeedsOneAgent() || r.dynakube.NeedsActiveGate()) { if meta.FindStatusCondition(*r.dynakube.Conditions(), PullSecretConditionType) == nil { return nil // no condition == nothing is there to clean up diff --git a/pkg/controllers/dynakube/oneagent/daemonset/daemonset.go b/pkg/controllers/dynakube/oneagent/daemonset/daemonset.go index e32c27d8c6..6fe02f70a6 100644 --- a/pkg/controllers/dynakube/oneagent/daemonset/daemonset.go +++ b/pkg/controllers/dynakube/oneagent/daemonset/daemonset.go @@ -307,7 +307,14 @@ func (b *builder) imagePullSecrets() []corev1.LocalObjectReference { return []corev1.LocalObjectReference{} } - return []corev1.LocalObjectReference{{Name: b.dk.PullSecretName()}} + imagePullSecrets := make([]corev1.LocalObjectReference, 0) + for _, pullSecretName := range b.dk.PullSecretsNames() { + imagePullSecrets = append(imagePullSecrets, corev1.LocalObjectReference{ + Name: pullSecretName, + }) + } + + return imagePullSecrets } func (b *builder) securityContext() *corev1.SecurityContext { diff --git a/pkg/injection/codemodule/installer/image/installer.go b/pkg/injection/codemodule/installer/image/installer.go index 27b63570dd..8a07e81668 100644 --- a/pkg/injection/codemodule/installer/image/installer.go +++ b/pkg/injection/codemodule/installer/image/installer.go @@ -30,7 +30,6 @@ type Properties struct { } func NewImageInstaller(ctx context.Context, fs afero.Fs, props *Properties) (installer.Installer, error) { - pullSecret := props.Dynakube.PullSecretWithoutData() defaultTransport := http.DefaultTransport.(*http.Transport).Clone() transport, err := registry.PrepareTransportForDynaKube(ctx, props.ApiReader, defaultTransport, props.Dynakube) @@ -38,7 +37,7 @@ func NewImageInstaller(ctx context.Context, fs afero.Fs, props *Properties) (ins return nil, err } - keychain, err := dockerkeychain.NewDockerKeychain(ctx, props.ApiReader, pullSecret) + keychain, err := dockerkeychain.NewDockerKeychains(ctx, props.ApiReader, props.Dynakube.Namespace, props.Dynakube.PullSecretsNames()) if err != nil { return nil, err } diff --git a/pkg/injection/codemodule/installer/image/installer_test.go b/pkg/injection/codemodule/installer/image/installer_test.go index baf585d8c8..94547a293c 100644 --- a/pkg/injection/codemodule/installer/image/installer_test.go +++ b/pkg/injection/codemodule/installer/image/installer_test.go @@ -69,7 +69,12 @@ func TestNewImageInstaller(t *testing.T) { }, Spec: dynatracev1beta2.DynaKubeSpec{}, } - pullSecret := dynakube.PullSecretWithoutData() + pullSecret := corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: dynakube.PullSecretName(), + Namespace: dynakube.Namespace, + }, + } pullSecret.Data = map[string][]byte{ corev1.DockerConfigJsonKey: []byte(emptyDockerConfig), } diff --git a/pkg/oci/dockerkeychain/docker_keychain.go b/pkg/oci/dockerkeychain/docker_keychain.go index aaad2676fb..0f42a85c2d 100644 --- a/pkg/oci/dockerkeychain/docker_keychain.go +++ b/pkg/oci/dockerkeychain/docker_keychain.go @@ -11,6 +11,7 @@ import ( "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/name" "github.com/pkg/errors" + "golang.org/x/exp/maps" corev1 "k8s.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -20,6 +21,58 @@ type DockerKeychain struct { mutex sync.Mutex } +func NewDockerKeychains(ctx context.Context, apiReader client.Reader, namespaceName string, pullSecretsNames []string) (authn.Keychain, error) { + keychain := &DockerKeychain{} + err := keychain.loadDockerConfigFromSecrets(ctx, apiReader, namespaceName, pullSecretsNames) + + return keychain, err +} + +func (keychain *DockerKeychain) loadDockerConfigFromSecrets(ctx context.Context, apiReader client.Reader, namespaceName string, pullSecretsNames []string) error { + if len(pullSecretsNames) == 0 { + return nil + } + + // it could be implementation of `LoadFromReader(configData ...io.Reader) function, see: + // - https://github.com/docker/cli/blob/master/cli/config/config.go#L106 + // - https://github.com/docker/cli/blob/master/cli/config/configfile/file.go#L83 + // dockerAuth(s) loaded by configFile.LoadFromReader(...) from different secrets are stored in a single instance of configFile.AuthConfigs map + // configFile.LoadFromReader(...) basically does `map[k] = v` so it works for many secrets + configFile := configfile.ConfigFile{ + AuthConfigs: make(map[string]dockertypes.AuthConfig), + } + + for _, pullSecretName := range pullSecretsNames { + log.Info("### Auth", "keys", maps.Keys(configFile.AuthConfigs)) + + pullSecret := corev1.Secret{} + + if err := apiReader.Get(ctx, client.ObjectKey{Namespace: namespaceName, Name: pullSecretName}, &pullSecret); err != nil { + log.Info("failed to load registry pull secret", "name", pullSecretName, "namespace", namespaceName) + + return errors.WithStack(err) + } + + dockerAuths, err := extractDockerAuthsFromSecret(&pullSecret) + if err != nil { + log.Info("failed to parse pull secret content", "name", pullSecret.Name, "namespace", pullSecret.Namespace) + + return err + } + + err = configFile.LoadFromReader(bytes.NewReader(dockerAuths)) + if err != nil { + return errors.WithStack(err) + } + } + + log.Info("### Auth", "keys", maps.Keys(configFile.AuthConfigs)) + + keychain.dockerConfig = &configFile + + return nil +} + func NewDockerKeychain(ctx context.Context, apiReader client.Reader, pullSecret corev1.Secret) (authn.Keychain, error) { keychain := &DockerKeychain{} err := keychain.loadDockerConfigFromSecret(ctx, apiReader, pullSecret) @@ -112,6 +165,8 @@ func (keychain *DockerKeychain) Resolve(target authn.Resource) (authn.Authentica return authn.Anonymous, nil } + log.Info("### Resolve", "target", target.String(), "targetRegistry", target.RegistryStr(), "auth", cfg.Auth) + return authn.FromConfig(authn.AuthConfig{ Username: cfg.Username, Password: cfg.Password,