From b75fa29292d549b7a2be60b69cb4600800659b8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Haram=20Nyg=C3=A5rd?= Date: Wed, 12 Feb 2025 10:51:05 +0100 Subject: [PATCH] refactor --- api/v1alpha1/digdirator/digdirator.go | 30 +++++-- api/v1alpha1/digdirator/idporten.go | 28 ------- api/v1alpha1/digdirator/maskinporten.go | 35 +------- internal/controllers/application.go | 102 +++++++++++------------- pkg/util/helperfunctions.go | 10 +-- 5 files changed, 76 insertions(+), 129 deletions(-) diff --git a/api/v1alpha1/digdirator/digdirator.go b/api/v1alpha1/digdirator/digdirator.go index 0fd16506..b8289256 100644 --- a/api/v1alpha1/digdirator/digdirator.go +++ b/api/v1alpha1/digdirator/digdirator.go @@ -1,6 +1,7 @@ package digdirator import ( + nais_io_v1 "github.com/nais/liberator/pkg/apis/nais.io/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -13,11 +14,6 @@ type DigdiratorURIs struct { ClientID string } -type DigdiratorClients struct { - IdPortenClient IdPortenClient - MaskinPortenClient MaskinportenClient -} - type DigdiratorProvider interface { IsEnabled() bool GetDigdiratorName() DigdiratorName @@ -26,7 +22,25 @@ type DigdiratorProvider interface { GetIssuerKey() string GetJwksKey() string GetClientIDKey() string - GetDigdiratorClientOwnerRef(digdiratorClients DigdiratorClients) (*[]v1.OwnerReference, error) - GetGeneratedDigdiratorSecret(digdiratorClients DigdiratorClients) (*string, error) - HandleDigdiratorClientError(digdiratorClients DigdiratorClients) error +} + +type DigdiratorClient interface { + GetOwnerReferences() []v1.OwnerReference + GetSecretName() string +} + +type MaskinportenClient struct { + nais_io_v1.MaskinportenClient +} + +func (m *MaskinportenClient) GetSecretName() string { + return m.Spec.SecretName +} + +type IDPortenClient struct { + nais_io_v1.IDPortenClient +} + +func (i *IDPortenClient) GetSecretName() string { + return i.Spec.SecretName } diff --git a/api/v1alpha1/digdirator/idporten.go b/api/v1alpha1/digdirator/idporten.go index fbc422fb..2680b0aa 100644 --- a/api/v1alpha1/digdirator/idporten.go +++ b/api/v1alpha1/digdirator/idporten.go @@ -1,11 +1,9 @@ package digdirator import ( - "fmt" "github.com/kartverket/skiperator/api/v1alpha1/istiotypes" "github.com/nais/digdirator/pkg/secrets" nais_io_v1 "github.com/nais/liberator/pkg/apis/nais.io/v1" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // Based off NAIS' IDPorten specification as seen here: @@ -126,29 +124,3 @@ func (i *IDPorten) GetJwksKey() string { func (i *IDPorten) GetClientIDKey() string { return secrets.IDPortenClientIDKey } - -func (i *IDPorten) GetDigdiratorClientOwnerRef(digdiratorClients DigdiratorClients) (*[]v1.OwnerReference, error) { - err := i.HandleDigdiratorClientError(digdiratorClients) - if err != nil { - return nil, err - } - return &digdiratorClients.IdPortenClient.Client.OwnerReferences, nil -} - -func (i *IDPorten) GetGeneratedDigdiratorSecret(digdiratorClients DigdiratorClients) (*string, error) { - err := i.HandleDigdiratorClientError(digdiratorClients) - if err != nil { - return nil, err - } - return &digdiratorClients.IdPortenClient.Client.Spec.SecretName, nil -} - -func (i *IDPorten) HandleDigdiratorClientError(digdiratorClients DigdiratorClients) error { - if digdiratorClients.IdPortenClient.Error != nil { - return fmt.Errorf("failed to get IDPortenClient: %w", digdiratorClients.IdPortenClient.Error) - } - if digdiratorClients.IdPortenClient.Client == nil { - return fmt.Errorf("IDPortenClient not found") - } - return nil -} diff --git a/api/v1alpha1/digdirator/maskinporten.go b/api/v1alpha1/digdirator/maskinporten.go index 24338ff3..c794bbb9 100644 --- a/api/v1alpha1/digdirator/maskinporten.go +++ b/api/v1alpha1/digdirator/maskinporten.go @@ -1,11 +1,9 @@ package digdirator import ( - "fmt" "github.com/kartverket/skiperator/api/v1alpha1/istiotypes" "github.com/nais/digdirator/pkg/secrets" nais_io_v1 "github.com/nais/liberator/pkg/apis/nais.io/v1" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // https://github.com/nais/liberator/blob/c9da4cf48a52c9594afc8a4325ff49bbd359d9d2/pkg/apis/nais.io/v1/naiserator_types.go#L376 @@ -26,16 +24,11 @@ type Maskinporten struct { Authentication *istiotypes.Authentication `json:"authentication,omitempty"` } -type MaskinportenClient struct { - Client *nais_io_v1.MaskinportenClient - Error error -} - const MaskinPortenName = "maskinporten" func (i *Maskinporten) IsEnabled() bool { return i != nil && i.Enabled && i.Authentication != nil && i.Authentication.Enabled - + } func (i *Maskinporten) GetDigdiratorName() DigdiratorName { @@ -67,29 +60,3 @@ func (i *Maskinporten) GetJwksKey() string { func (i *Maskinporten) GetClientIDKey() string { return secrets.MaskinportenClientIDKey } - -func (i *Maskinporten) GetDigdiratorClientOwnerRef(digdiratorClients DigdiratorClients) (*[]v1.OwnerReference, error) { - err := i.HandleDigdiratorClientError(digdiratorClients) - if err != nil { - return nil, err - } - return &digdiratorClients.MaskinPortenClient.Client.OwnerReferences, nil -} - -func (i *Maskinporten) GetGeneratedDigdiratorSecret(digdiratorClients DigdiratorClients) (*string, error) { - err := i.HandleDigdiratorClientError(digdiratorClients) - if err != nil { - return nil, err - } - return &digdiratorClients.MaskinPortenClient.Client.Spec.SecretName, nil -} - -func (i *Maskinporten) HandleDigdiratorClientError(digdiratorClients DigdiratorClients) error { - if digdiratorClients.MaskinPortenClient.Error != nil { - return fmt.Errorf("failed to get MaskinportenClient: %w", digdiratorClients.MaskinPortenClient.Error) - } - if digdiratorClients.MaskinPortenClient.Client == nil { - return fmt.Errorf("MaskinportenClient not found") - } - return nil -} diff --git a/internal/controllers/application.go b/internal/controllers/application.go index f2ab912e..86e31520 100644 --- a/internal/controllers/application.go +++ b/internal/controllers/application.go @@ -192,7 +192,7 @@ func (r *ApplicationReconciler) Reconcile(ctx context.Context, req reconcile.Req if err != nil { rLog.Error(err, "cant find identity config map") } //TODO Error state? - authConfigs, err := r.GetAuthConfigsForApplication(ctx, application) + authConfigs, err := r.getAuthConfigsForApplication(ctx, application) if err != nil { rLog.Error(err, "can't resolve auth config") } @@ -421,13 +421,14 @@ func validateIngresses(application *skiperatorv1alpha1.Application) error { return nil } -func (r *ApplicationReconciler) GetAuthConfigsForApplication(ctx context.Context, application *skiperatorv1alpha1.Application) (*AuthConfigs, error) { - if application == nil { - return nil, fmt.Errorf("cannot retrieve AuthConfigs for nil application") - } +func (r *ApplicationReconciler) getAuthConfigsForApplication(ctx context.Context, application *skiperatorv1alpha1.Application) (*AuthConfigs, error) { var authConfigs AuthConfigs - for _, provider := range getDigdiratorImplementations(*application) { + providers := []digdirator.DigdiratorProvider{ + application.Spec.IDPorten, + application.Spec.Maskinporten, + } + for _, provider := range providers { if provider.IsEnabled() { authConfig, err := r.getAuthConfig(ctx, *application, provider) if err != nil { @@ -444,13 +445,6 @@ func (r *ApplicationReconciler) GetAuthConfigsForApplication(ctx context.Context } } -func getDigdiratorImplementations(application skiperatorv1alpha1.Application) []digdirator.DigdiratorProvider { - return []digdirator.DigdiratorProvider{ - application.Spec.IDPorten, - application.Spec.Maskinporten, - } -} - func (r *ApplicationReconciler) getAuthConfig(ctx context.Context, application skiperatorv1alpha1.Application, digdiratorProvider digdirator.DigdiratorProvider) (*AuthConfig, error) { secret, err := r.getAuthConfigSecret(ctx, application, digdiratorProvider) if err != nil { @@ -468,60 +462,60 @@ func (r *ApplicationReconciler) getAuthConfig(ctx context.Context, application s } func (r *ApplicationReconciler) getAuthConfigSecret(ctx context.Context, application skiperatorv1alpha1.Application, digdiratorProvider digdirator.DigdiratorProvider) (*corev1.Secret, error) { - namespacedName := types.NamespacedName{ - Namespace: application.Namespace, - } + var secretName *string + var err error + if digdiratorProvider.GetProvidedSecretName() != nil { - namespacedName.Name = *digdiratorProvider.GetProvidedSecretName() - secret, err := util.GetSecret(r.GetClient(), ctx, namespacedName) + secretName = digdiratorProvider.GetProvidedSecretName() + } else { + secretName, err = r.getDigdiratorSecretName(ctx, digdiratorProvider, application) if err != nil { - return nil, fmt.Errorf("failed to retrieve provided digdiratorProvider secret: '%s': %w", namespacedName, err) + return nil, err } - return &secret, nil } - namespacedName.Name = application.Name - digdiratorClients := r.getDigdiratorClients(ctx, namespacedName) - ownerReferences, err := digdiratorProvider.GetDigdiratorClientOwnerRef(digdiratorClients) - if err != nil { - return nil, fmt.Errorf("failed to retrieve ownerRefs from digdiratorClient %s: %w", namespacedName, err) + namespacedName := types.NamespacedName{ + Name: *secretName, + Namespace: application.Namespace, } - secretName, err := digdiratorProvider.GetGeneratedDigdiratorSecret(digdiratorClients) + + secret, err := util.GetSecret(r.GetClient(), ctx, namespacedName) if err != nil { - return nil, fmt.Errorf("failed to retrieve secret from digdiratorClient %s: %w", namespacedName, err) + return nil, err } - return r.getDigdiratorExistingSecret(ctx, application, *ownerReferences, *secretName) + return &secret, nil + } -func (r *ApplicationReconciler) getDigdiratorClients(ctx context.Context, namespacedName types.NamespacedName) digdirator.DigdiratorClients { - idPortenClient, idPortenClientError := util.GetIdPortenClient(r.GetClient(), ctx, namespacedName) - maskinportenClient, maskinportenClientError := util.GetMaskinportenClient(r.GetClient(), ctx, namespacedName) - return digdirator.DigdiratorClients{ - IdPortenClient: digdirator.IdPortenClient{ - Client: idPortenClient, - Error: idPortenClientError, - }, - MaskinPortenClient: digdirator.MaskinportenClient{ - Client: maskinportenClient, - Error: maskinportenClientError, - }, +func (r *ApplicationReconciler) getDigdiratorSecretName(ctx context.Context, digdiratorProvider digdirator.DigdiratorProvider, application skiperatorv1alpha1.Application) (*string, error) { + var digdiratorClient digdirator.DigdiratorClient + var err error + + namespacedName := types.NamespacedName{ + Name: application.Name, + Namespace: application.Namespace, } -} -func (r *ApplicationReconciler) getDigdiratorExistingSecret(ctx context.Context, application skiperatorv1alpha1.Application, ownerReferences []metav1.OwnerReference, secretName string) (*corev1.Secret, error) { - for _, ownerReference := range ownerReferences { - if ownerReference.UID == application.UID { - secretName := types.NamespacedName{ - Namespace: application.Namespace, - Name: secretName, - } - secret, err := util.GetSecret(r.GetClient(), ctx, secretName) - if err != nil { - return nil, fmt.Errorf("failed to retrieve existing digdirator secret: '%s': %w", secretName.String(), err) - } - return &secret, nil + if digdiratorProvider.GetDigdiratorName() == digdirator.MaskinPortenName { + digdiratorClient, err = util.GetMaskinportenClient(r.GetClient(), ctx, namespacedName) + if err != nil { + return nil, err + } + } else { + digdiratorClient, err = util.GetIdPortenClient(r.GetClient(), ctx, namespacedName) + if err != nil { + return nil, err } } - return nil, nil + ownershipRefs := digdiratorClient.GetOwnerReferences() + secretName := digdiratorClient.GetSecretName() + + for _, ownershipRef := range ownershipRefs { + if ownershipRef.UID == application.UID { + return &secretName, nil + } + } + + return nil, fmt.Errorf("digdirator client doesn't exist: %s", namespacedName) } diff --git a/pkg/util/helperfunctions.go b/pkg/util/helperfunctions.go index dc751086..79a9ef04 100644 --- a/pkg/util/helperfunctions.go +++ b/pkg/util/helperfunctions.go @@ -4,9 +4,9 @@ import ( "context" "fmt" skiperatorv1alpha1 "github.com/kartverket/skiperator/api/v1alpha1" + "github.com/kartverket/skiperator/api/v1alpha1/digdirator" "github.com/kartverket/skiperator/api/v1alpha1/podtypes" "github.com/mitchellh/hashstructure/v2" - nais_io_v1 "github.com/nais/liberator/pkg/apis/nais.io/v1" "github.com/nais/liberator/pkg/namegen" "hash/fnv" corev1 "k8s.io/api/core/v1" @@ -67,8 +67,8 @@ func GetSecret(client client.Client, ctx context.Context, namespacedName types.N return secret, err } -func GetIdPortenClient(k8sClient client.Client, ctx context.Context, namespacedName types.NamespacedName) (*nais_io_v1.IDPortenClient, error) { - idPortenClient := &nais_io_v1.IDPortenClient{} +func GetIdPortenClient(k8sClient client.Client, ctx context.Context, namespacedName types.NamespacedName) (*digdirator.IDPortenClient, error) { + idPortenClient := &digdirator.IDPortenClient{} if err := k8sClient.Get(ctx, namespacedName, idPortenClient); err != nil { if errors.IsNotFound(err) { return nil, nil @@ -78,8 +78,8 @@ func GetIdPortenClient(k8sClient client.Client, ctx context.Context, namespacedN return idPortenClient, nil } -func GetMaskinportenClient(k8sClient client.Client, ctx context.Context, namespacedName types.NamespacedName) (*nais_io_v1.MaskinportenClient, error) { - maskinPortenClient := &nais_io_v1.MaskinportenClient{} +func GetMaskinportenClient(k8sClient client.Client, ctx context.Context, namespacedName types.NamespacedName) (*digdirator.MaskinportenClient, error) { + maskinPortenClient := &digdirator.MaskinportenClient{} if err := k8sClient.Get(ctx, namespacedName, maskinPortenClient); err != nil { if errors.IsNotFound(err) { return nil, nil