From 36b77d40875ff14e9498d21746370cf969502cb6 Mon Sep 17 00:00:00 2001 From: Marco Bebway Date: Thu, 8 Feb 2024 13:11:54 +0100 Subject: [PATCH] Fix lint issues (#116) * Fix lint issues for goerr113 * Fix lint issues for gochecknoinits * Fix lint issues for gofumpt * Fix lint issues for gochecknoglobals * Fix lint issues for stylecheck * Fix lint issues for funlen * Fix lint issues for nestif --- api/v1alpha1/eventingauth_types.go | 6 +- api/v1alpha1/groupversion_info.go | 10 +- api/v1alpha1/status.go | 4 +- api/v1alpha1/status_test.go | 13 +- cmd/main.go | 28 +-- cmd/main_test.go | 12 ++ controllers/eventingauth_controller.go | 93 +++++---- controllers/eventingauth_controller_test.go | 5 +- controllers/kyma_controller.go | 6 +- controllers/kyma_controller_test.go | 2 +- controllers/stubs_test.go | 16 +- controllers/suite_test.go | 10 +- internal/ias/client.go | 86 ++++---- internal/ias/client_test.go | 188 +++++++++--------- internal/ias/ias_config.go | 27 ++- internal/ias/ias_config_test.go | 35 ++-- .../api/SCI_Application_Directory.yaml | 32 +-- internal/ias/internal/api/ias.gen.go | 14 +- internal/ias/internal/oidc/oidc-client.go | 12 +- .../ias/internal/oidc/oidc-client_test.go | 16 +- internal/ias/types.go | 22 +- internal/skr/client.go | 7 +- internal/skr/client_test.go | 22 +- 23 files changed, 350 insertions(+), 316 deletions(-) create mode 100644 cmd/main_test.go diff --git a/api/v1alpha1/eventingauth_types.go b/api/v1alpha1/eventingauth_types.go index a538b8f..b83f759 100644 --- a/api/v1alpha1/eventingauth_types.go +++ b/api/v1alpha1/eventingauth_types.go @@ -60,7 +60,7 @@ type AuthSecret struct { // NamespacedName of the secret on the managed runtime cluster NamespacedName string `json:"namespacedName"` // Runtime ID of the cluster where the secret is created - ClusterId string `json:"clusterId"` + ClusterID string `json:"clusterId"` } //+kubebuilder:object:root=true @@ -85,6 +85,6 @@ type EventingAuthList struct { Items []EventingAuth `json:"items"` } -func init() { - SchemeBuilder.Register(&EventingAuth{}, &EventingAuthList{}) +func init() { //nolint:gochecknoinits // Used on the package level. + schemeBuilder.Register(&EventingAuth{}, &EventingAuthList{}) } diff --git a/api/v1alpha1/groupversion_info.go b/api/v1alpha1/groupversion_info.go index 34c9e3d..aa0f28c 100644 --- a/api/v1alpha1/groupversion_info.go +++ b/api/v1alpha1/groupversion_info.go @@ -25,12 +25,12 @@ import ( ) var ( - // GroupVersion is group version used to register these objects. - GroupVersion = schema.GroupVersion{Group: "operator.kyma-project.io", Version: "v1alpha1"} + // groupVersion is group version used to register these objects. + groupVersion = schema.GroupVersion{Group: "operator.kyma-project.io", Version: "v1alpha1"} //nolint:gochecknoglobals // Used internally in the package. - // SchemeBuilder is used to add go types to the GroupVersionKind scheme. - SchemeBuilder = &scheme.Builder{GroupVersion: GroupVersion} + // schemeBuilder is used to add go types to the GroupVersionKind scheme. + schemeBuilder = &scheme.Builder{GroupVersion: groupVersion} //nolint:gochecknoglobals // Used internally in the package. // AddToScheme adds the types in this group-version to the given scheme. - AddToScheme = SchemeBuilder.AddToScheme + AddToScheme = schemeBuilder.AddToScheme //nolint:gochecknoglobals // Used outside the package. ) diff --git a/api/v1alpha1/status.go b/api/v1alpha1/status.go index 378348c..e7ec163 100644 --- a/api/v1alpha1/status.go +++ b/api/v1alpha1/status.go @@ -1,9 +1,9 @@ package v1alpha1 import ( - "fmt" "reflect" + "github.com/pkg/errors" kmetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -39,7 +39,7 @@ func UpdateConditionAndState(eventingAuth *EventingAuth, conditionType Condition eventingAuth.Status.Conditions = MakeSecretReadyCondition(eventingAuth, err) } default: - return eventingAuth.Status, fmt.Errorf("unsupported condition type: %s", conditionType) + return eventingAuth.Status, errors.Errorf("unsupported condition type: %s", conditionType) } if err != nil { diff --git a/api/v1alpha1/status_test.go b/api/v1alpha1/status_test.go index eeda62a..93a8327 100644 --- a/api/v1alpha1/status_test.go +++ b/api/v1alpha1/status_test.go @@ -1,9 +1,9 @@ package v1alpha1 import ( - "fmt" "testing" + "github.com/pkg/errors" "github.com/stretchr/testify/require" kmetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -162,7 +162,7 @@ func Test_MakeApplicationReadyCondition(t *testing.T) { Message: ConditionMessageApplicationCreated, }, }}), - givenErr: fmt.Errorf(mockErrorMessage), + givenErr: errors.Errorf(mockErrorMessage), wantConditions: []kmetav1.Condition{ { Type: string(ConditionApplicationReady), @@ -262,7 +262,7 @@ func Test_MakeSecretReadyCondition(t *testing.T) { Message: ConditionMessageSecretCreated, }, }}), - givenErr: fmt.Errorf(mockErrorMessage), + givenErr: errors.Errorf(mockErrorMessage), wantConditions: []kmetav1.Condition{ { Type: string(ConditionApplicationReady), @@ -319,7 +319,7 @@ func Test_UpdateConditionAndState(t *testing.T) { State: StateReady, }), conditionType: ConditionSecretReady, - givenErr: fmt.Errorf(mockErrorMessage), + givenErr: errors.Errorf(mockErrorMessage), wantStatus: EventingAuthStatus{ Conditions: []kmetav1.Condition{ { @@ -416,7 +416,7 @@ func Test_UpdateConditionAndState(t *testing.T) { State: StateNotReady, }), conditionType: invalidConditionType, - wantError: fmt.Errorf("unsupported condition type: %s", invalidConditionType), + wantError: errors.Errorf("unsupported condition type: %s", invalidConditionType), }, } @@ -448,6 +448,7 @@ func createTwoTrueConditions() []kmetav1.Condition { }, } } + func createTwoFalseConditions() []kmetav1.Condition { return []kmetav1.Condition{ { @@ -492,7 +493,7 @@ func createEventingAuthStatus(secretReadyStatus kmetav1.ConditionStatus, appName }, AuthSecret: &AuthSecret{ NamespacedName: secretNSName, - ClusterId: "mock-cluster-reference", + ClusterID: "mock-cluster-reference", }, State: state, } diff --git a/cmd/main.go b/cmd/main.go index 559b593..18459c9 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -37,23 +37,9 @@ import ( _ "k8s.io/client-go/plugin/pkg/client/auth" ) -const webhookPort = 9443 - -var ( - scheme = runtime.NewScheme() - setupLog = kcontrollerruntime.Log.WithName("setup") -) - -func init() { - kutilruntime.Must(kscheme.AddToScheme(scheme)) - - kutilruntime.Must(klmapiv1beta1.AddToScheme(scheme)) - - kutilruntime.Must(eamapiv1alpha1.AddToScheme(scheme)) - //+kubebuilder:scaffold:scheme -} - func main() { + const webhookPort = 9443 + setupLog := kcontrollerruntime.Log.WithName("setup") var metricsAddr string var enableLeaderElection bool var probeAddr string @@ -71,7 +57,7 @@ func main() { kcontrollerruntime.SetLogger(zap.New(zap.UseFlagOptions(&opts))) mgr, err := kcontrollerruntime.NewManager(kcontrollerruntime.GetConfigOrDie(), kcontrollerruntime.Options{ - Scheme: scheme, + Scheme: initScheme(), HealthProbeBindAddress: probeAddr, LeaderElection: enableLeaderElection, LeaderElectionID: "210590f8.kyma-project.io", @@ -127,3 +113,11 @@ func main() { os.Exit(1) } } + +func initScheme() *runtime.Scheme { + scheme := runtime.NewScheme() + kutilruntime.Must(kscheme.AddToScheme(scheme)) + kutilruntime.Must(klmapiv1beta1.AddToScheme(scheme)) + kutilruntime.Must(eamapiv1alpha1.AddToScheme(scheme)) + return scheme +} diff --git a/cmd/main_test.go b/cmd/main_test.go new file mode 100644 index 0000000..5e72cbb --- /dev/null +++ b/cmd/main_test.go @@ -0,0 +1,12 @@ +package main + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func Test_initScheme(t *testing.T) { + scheme := initScheme() + require.NotNil(t, scheme) +} diff --git a/controllers/eventingauth_controller.go b/controllers/eventingauth_controller.go index 287e9d6..5c0b653 100644 --- a/controllers/eventingauth_controller.go +++ b/controllers/eventingauth_controller.go @@ -22,9 +22,11 @@ import ( "os" "reflect" + "github.com/go-logr/logr" eamapiv1alpha1 "github.com/kyma-project/eventing-auth-manager/api/v1alpha1" eamias "github.com/kyma-project/eventing-auth-manager/internal/ias" "github.com/kyma-project/eventing-auth-manager/internal/skr" + "github.com/pkg/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" kcontrollerruntime "sigs.k8s.io/controller-runtime" @@ -91,6 +93,10 @@ func (r *eventingAuthReconciler) Reconcile(ctx context.Context, req kcontrollerr return kcontrollerruntime.Result{}, nil } + return r.handleApplicationSecret(ctx, logger, cr) +} + +func (r *eventingAuthReconciler) handleApplicationSecret(ctx context.Context, logger logr.Logger, cr eamapiv1alpha1.EventingAuth) (kcontrollerruntime.Result, error) { skrClient, err := skr.NewClient(r.Client, cr.Name) if err != nil { logger.Error(err, "Failed to retrieve client of target cluster") @@ -102,53 +108,56 @@ func (r *eventingAuthReconciler) Reconcile(ctx context.Context, req kcontrollerr logger.Error(err, "Failed to retrieve secret state from target cluster") return kcontrollerruntime.Result{}, err } + if appSecretExists { + logger.Info("Reconciliation done, Application secret already exists") + return kcontrollerruntime.Result{}, nil + } // TODO: check if secret creation condition is also true, otherwise it never updates false secret ready condition - if !appSecretExists { - var createAppErr error - iasApplication, appExists := r.existingIasApplications[cr.Name] - if !appExists { - logger.Info("Creating application in IAS") - iasApplication, createAppErr = r.iasClient.CreateApplication(ctx, cr.Name) - if createAppErr != nil { - logger.Error(createAppErr, "Failed to create application in IAS") - if err := r.updateEventingAuthStatus(ctx, &cr, eamapiv1alpha1.ConditionApplicationReady, createAppErr); err != nil { - return kcontrollerruntime.Result{}, err - } - return kcontrollerruntime.Result{}, createAppErr - } - logger.Info("Successfully created application in IAS") - r.existingIasApplications[cr.Name] = iasApplication - } - cr.Status.Application = &eamapiv1alpha1.IASApplication{ - Name: cr.Name, - UUID: iasApplication.GetId(), - } - if err := r.updateEventingAuthStatus(ctx, &cr, eamapiv1alpha1.ConditionApplicationReady, nil); err != nil { - return kcontrollerruntime.Result{}, err - } - logger.Info("Creating application secret on SKR") - appSecret, createSecretErr := skrClient.CreateSecret(ctx, iasApplication) - if createSecretErr != nil { - logger.Error(createSecretErr, "Failed to create application secret on SKR") - if err := r.updateEventingAuthStatus(ctx, &cr, eamapiv1alpha1.ConditionSecretReady, createSecretErr); err != nil { + iasApplication, appExists := r.existingIasApplications[cr.Name] + if !appExists { + var createAppErr error + logger.Info("Creating application in IAS") + iasApplication, createAppErr = r.iasClient.CreateApplication(ctx, cr.Name) + if createAppErr != nil { + logger.Error(createAppErr, "Failed to create application in IAS") + if err := r.updateEventingAuthStatus(ctx, &cr, eamapiv1alpha1.ConditionApplicationReady, createAppErr); err != nil { return kcontrollerruntime.Result{}, err } return kcontrollerruntime.Result{}, createAppErr } - logger.Info("Successfully created application secret on SKR") - - // Because the application secret is created on the SKR, we can delete it from the cache. - delete(r.existingIasApplications, cr.Name) + logger.Info("Successfully created application in IAS") + r.existingIasApplications[cr.Name] = iasApplication + } + cr.Status.Application = &eamapiv1alpha1.IASApplication{ + Name: cr.Name, + UUID: iasApplication.GetID(), + } + if err := r.updateEventingAuthStatus(ctx, &cr, eamapiv1alpha1.ConditionApplicationReady, nil); err != nil { + return kcontrollerruntime.Result{}, err + } - cr.Status.AuthSecret = &eamapiv1alpha1.AuthSecret{ - ClusterId: cr.Name, - NamespacedName: fmt.Sprintf("%s/%s", appSecret.Namespace, appSecret.Name), - } - if err := r.updateEventingAuthStatus(ctx, &cr, eamapiv1alpha1.ConditionSecretReady, nil); err != nil { + logger.Info("Creating application secret on SKR") + appSecret, createSecretErr := skrClient.CreateSecret(ctx, iasApplication) + if createSecretErr != nil { + logger.Error(createSecretErr, "Failed to create application secret on SKR") + if err := r.updateEventingAuthStatus(ctx, &cr, eamapiv1alpha1.ConditionSecretReady, createSecretErr); err != nil { return kcontrollerruntime.Result{}, err } + return kcontrollerruntime.Result{}, createSecretErr + } + logger.Info("Successfully created application secret on SKR") + + // Because the application secret is created on the SKR, we can delete it from the cache. + delete(r.existingIasApplications, cr.Name) + + cr.Status.AuthSecret = &eamapiv1alpha1.AuthSecret{ + ClusterID: cr.Name, + NamespacedName: fmt.Sprintf("%s/%s", appSecret.Namespace, appSecret.Name), + } + if err := r.updateEventingAuthStatus(ctx, &cr, eamapiv1alpha1.ConditionSecretReady, nil); err != nil { + return kcontrollerruntime.Result{}, err } logger.Info("Reconciliation done") @@ -168,7 +177,7 @@ func (r *eventingAuthReconciler) getIasClient() (eamias.Client, error) { // update IAS client if credentials are changed iasClient, err := eamias.NewClient(newIasCredentials.URL, newIasCredentials.Username, newIasCredentials.Password) if err != nil { - return nil, fmt.Errorf("failed to createa a new IAS client: %w", err) + return nil, errors.Wrap(err, "failed to create a new IAS client") } return iasClient, nil } @@ -191,7 +200,7 @@ func (r *eventingAuthReconciler) addFinalizer(ctx context.Context, cr *eamapiv1a kcontrollerruntime.Log.Info("Adding finalizer", "eventingAuth", cr.Name, "eventingAuthNamespace", cr.Namespace) controllerutil.AddFinalizer(cr, eventingAuthFinalizerName) if err := r.Update(ctx, cr); err != nil { - return fmt.Errorf("failed to add finalizer: %w", err) + return errors.Wrap(err, "failed to add finalizer") } } return nil @@ -203,7 +212,7 @@ func (r *eventingAuthReconciler) handleDeletion(ctx context.Context, iasClient e if controllerutil.ContainsFinalizer(cr, eventingAuthFinalizerName) { // delete IAS application clean-up if err := iasClient.DeleteApplication(ctx, cr.Name); err != nil { - return fmt.Errorf("failed to delete IAS Application: %w", err) + return errors.Wrap(err, "failed to delete IAS Application") } kcontrollerruntime.Log.Info("Deleted IAS application", "eventingAuth", cr.Name, "namespace", cr.Namespace) @@ -218,7 +227,7 @@ func (r *eventingAuthReconciler) handleDeletion(ctx context.Context, iasClient e // remove our finalizer from the list and update it. controllerutil.RemoveFinalizer(cr, eventingAuthFinalizerName) if err := r.Update(ctx, cr); err != nil { - return fmt.Errorf("failed to remove finalizer: %w", err) + return errors.Wrap(err, "failed to remove finalizer") } } return nil @@ -263,7 +272,7 @@ func (r *eventingAuthReconciler) updateEventingAuthStatus(ctx context.Context, c // sync EventingAuth status with k8s if err = r.updateStatus(ctx, actualEventingAuth, desiredEventingAuth); err != nil { - return fmt.Errorf("failed to update EventingAuth status: %w", err) + return errors.Wrap(err, "failed to update EventingAuth status") } return nil diff --git a/controllers/eventingauth_controller_test.go b/controllers/eventingauth_controller_test.go index 9fc7ded..0aa2b09 100644 --- a/controllers/eventingauth_controller_test.go +++ b/controllers/eventingauth_controller_test.go @@ -221,7 +221,7 @@ func verifyEventingAuthStatusNotReadyAppCreationFailed(cr *eamapiv1alpha1.Eventi string(eamapiv1alpha1.ConditionApplicationReady), kmetav1.ConditionFalse, eamapiv1alpha1.ConditionReasonApplicationCreationFailed, - "stubbed IAS application creation error"), + errIASApplicationCreation.Error()), )) }, defaultTimeout).Should(Succeed()) } @@ -239,7 +239,7 @@ func verifyEventingAuthStatusNotReadySecretCreationFailed(cr *eamapiv1alpha1.Eve string(eamapiv1alpha1.ConditionSecretReady), kmetav1.ConditionFalse, eamapiv1alpha1.ConditionReasonSecretCreationFailed, - "stubbed skr secret creation error"), + errSKRSecretCreation.Error()), )) }, defaultTimeout).Should(Succeed()) } @@ -252,6 +252,7 @@ func conditionMatcher(t string, s kmetav1.ConditionStatus, r, m string) onsigome "Message": Equal(m), }) } + func deleteEventingAuthAndVerify(e *eamapiv1alpha1.EventingAuth) { By(fmt.Sprintf("Deleting EventingAuth %s", e.Name)) if err := k8sClient.Get(context.TODO(), kpkgclient.ObjectKeyFromObject(e), &eamapiv1alpha1.EventingAuth{}); err != nil { diff --git a/controllers/kyma_controller.go b/controllers/kyma_controller.go index 911f869..96c77d9 100644 --- a/controllers/kyma_controller.go +++ b/controllers/kyma_controller.go @@ -2,11 +2,11 @@ package controllers import ( "context" - "fmt" "time" eamapiv1alpha1 "github.com/kyma-project/eventing-auth-manager/api/v1alpha1" klmapiv1beta1 "github.com/kyma-project/lifecycle-manager/api/v1beta1" + "github.com/pkg/errors" kapierrors "k8s.io/apimachinery/pkg/api/errors" kmetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -67,11 +67,11 @@ func (r *KymaReconciler) createEventingAuth(ctx context.Context, kyma *klmapiv1b } err = r.Client.Create(ctx, eventingAuth) if err != nil { - return fmt.Errorf("failed to create EventingAuth resource: %w", err) + return errors.Wrap(err, "failed to create EventingAuth resource") } return nil } - return fmt.Errorf("failed to retrieve EventingAuth resource: %w", err) + return errors.Wrap(err, "failed to retrieve EventingAuth resource") } return nil } diff --git a/controllers/kyma_controller_test.go b/controllers/kyma_controller_test.go index 7cca3a5..9ab7472 100644 --- a/controllers/kyma_controller_test.go +++ b/controllers/kyma_controller_test.go @@ -72,7 +72,7 @@ func verifyEventingAuth(namespace, name string) { g.Expect(eventingAuth.Status.Application.UUID).ShouldNot(BeEmpty()) g.Expect(eventingAuth.Status.AuthSecret).ShouldNot(BeNil()) g.Expect(eventingAuth.Status.AuthSecret.NamespacedName).To(Equal((skr.ApplicationSecretNamespace + "/" + skr.ApplicationSecretName))) - g.Expect(eventingAuth.Status.AuthSecret.ClusterId).ShouldNot(BeEmpty()) + g.Expect(eventingAuth.Status.AuthSecret.ClusterID).ShouldNot(BeEmpty()) }, defaultTimeout).Should(Succeed()) } diff --git a/controllers/stubs_test.go b/controllers/stubs_test.go index bf606eb..e37e1c9 100644 --- a/controllers/stubs_test.go +++ b/controllers/stubs_test.go @@ -18,6 +18,9 @@ var ( originalNewIasClientFunc func(iasTenantUrl, user, password string) (eamias.Client, error) originalReadCredentialsFunc func(namespace, name string, k8sClient client.Client) (*eamias.Credentials, error) originalNewSkrClientFunc func(k8sClient client.Client, targetClusterId string) (skr.Client, error) + + errIASApplicationCreation = errors.New("stubbed IAS application creation error") + errSKRSecretCreation = errors.New("stubbed skr secret creation error") ) func stubSuccessfulIasAppCreation() { @@ -33,12 +36,11 @@ func stubFailedIasAppCreation() { func stubIasAppCreation(c eamias.Client) { // The IAS client is initialized once in the "Reconcile" method of the controller. To update the IAS client stub by forcing a replacement, we need to // update the IAS credentials stub so that the implemented logic assumes that the IAS credentials have been rotated and forces a reinitialization of the IAS client. - replaceIasReadCredentialsWithStub(eamias.Credentials{URL: iasUrl, Username: uuid.New().String(), Password: iasPassword}) + replaceIasReadCredentialsWithStub(eamias.Credentials{URL: iasURL, Username: uuid.New().String(), Password: iasPassword}) replaceIasNewIasClientWithStub(c) } -type iasClientStub struct { -} +type iasClientStub struct{} func (i iasClientStub) CreateApplication(_ context.Context, name string) (eamias.Application, error) { return eamias.NewApplication( @@ -63,7 +65,7 @@ type appCreationFailsIasClientStub struct { } func (i appCreationFailsIasClientStub) CreateApplication(_ context.Context, _ string) (eamias.Application, error) { - return eamias.Application{}, errors.New("stubbed IAS application creation error") + return eamias.Application{}, errIASApplicationCreation } func replaceIasReadCredentialsWithStub(credentials eamias.Credentials) { @@ -77,12 +79,15 @@ func storeOriginalsOfStubbedFunctions() { originalNewIasClientFunc = eamias.NewClient originalNewSkrClientFunc = skr.NewClient } + func revertReadCredentialsStub() { eamias.ReadCredentials = originalReadCredentialsFunc } + func revertIasNewClientStub() { eamias.NewClient = originalNewIasClientFunc } + func revertSkrNewClientStub() { skr.NewClient = originalNewSkrClientFunc } @@ -100,6 +105,7 @@ type skrClientStub struct { func (s skrClientStub) CreateSecret(_ context.Context, app eamias.Application) (kcorev1.Secret, error) { return app.ToSecret(skr.ApplicationSecretName, skr.ApplicationSecretNamespace), nil } + func (s skrClientStub) HasApplicationSecret(_ context.Context) (bool, error) { return false, nil } @@ -123,7 +129,7 @@ type secretCreationFailedSkrClientStub struct { } func (s secretCreationFailedSkrClientStub) CreateSecret(_ context.Context, _ eamias.Application) (kcorev1.Secret, error) { - return kcorev1.Secret{}, errors.New("stubbed skr secret creation error") + return kcorev1.Secret{}, errSKRSecretCreation } func replaceSkrClientWithStub(c skr.Client) { diff --git a/controllers/suite_test.go b/controllers/suite_test.go index ce79067..88fd234 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -64,7 +64,7 @@ var ( targetClusterK8sClient client.Client testEnv *envtest.Environment targetClusterTestEnv *envtest.Environment - iasUrl string + iasURL string iasUsername string iasPassword string useExistingCluster bool @@ -82,7 +82,7 @@ var _ = BeforeSuite(func(specCtx SpecContext) { kpkglog.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) ctx, cancel = context.WithCancel(context.TODO()) - iasUrl = os.Getenv("TEST_EVENTING_AUTH_IAS_URL") + iasURL = os.Getenv("TEST_EVENTING_AUTH_IAS_URL") iasUsername = os.Getenv("TEST_EVENTING_AUTH_IAS_USER") iasPassword = os.Getenv("TEST_EVENTING_AUTH_IAS_PASSWORD") useExistingCluster = os.Getenv("USE_EXISTING_CLUSTER") == "true" @@ -128,7 +128,7 @@ var _ = BeforeSuite(func(specCtx SpecContext) { Expect(k8sClient.Create(context.TODO(), kcpNs)).Should(Succeed()) if existIasCreds() { - createIasCredsSecret(iasUrl, iasUsername, iasPassword) + createIasCredsSecret(iasURL, iasUsername, iasPassword) } testSyncPeriod := time.Second * 1 @@ -154,7 +154,6 @@ var _ = BeforeSuite(func(specCtx SpecContext) { defer GinkgoRecover() Expect(mgr.Start(ctx)).Should(Succeed()) }() - }, NodeTimeout(60*time.Second)) var _ = AfterSuite(func() { @@ -178,7 +177,6 @@ var _ = AfterSuite(func() { func stopTestEnv(env *envtest.Environment) { err := env.Stop() - // Suggested workaround for timeout issue https://github.com/kubernetes-sigs/controller-runtime/issues/1571#issuecomment-1005575071 if err != nil { time.Sleep(3 * time.Second) @@ -187,7 +185,7 @@ func stopTestEnv(env *envtest.Environment) { } func existIasCreds() bool { - return iasUrl != "" && iasUsername != "" && iasPassword != "" + return iasURL != "" && iasUsername != "" && iasPassword != "" } func initTargetClusterConfig() (client.Client, error) { diff --git a/internal/ias/client.go b/internal/ias/client.go index 35f2c61..f6c6659 100644 --- a/internal/ias/client.go +++ b/internal/ias/client.go @@ -15,33 +15,43 @@ import ( kcontrollerruntime "sigs.k8s.io/controller-runtime" ) +var ( + errCreateApplication = errors.New("failed to create application") + errFetchExistingApplications = errors.New("failed to fetch existing applications") + errDeleteExistingApplicationBeforeCreation = errors.New("failed to delete existing application before creation") + errCreateAPISecret = errors.New("failed to create api secret") + errRetrieveClientID = errors.New("failed to retrieve client ID") + errFetchTokenURL = errors.New("failed to fetch token url") + errFetchJWKSURI = errors.New("failed to fetch jwks uri") + errDeleteApplication = errors.New("failed to delete application") +) + type Client interface { CreateApplication(ctx context.Context, name string) (Application, error) DeleteApplication(ctx context.Context, name string) error GetCredentials() *Credentials } -var NewClient = func(iasTenantUrl, user, password string) (Client, error) { - +var NewClient = func(iasTenantUrl, user, password string) (Client, error) { //nolint:gochecknoglobals // For mocking purposes. basicAuthProvider, err := securityprovider.NewSecurityProviderBasicAuth(user, password) if err != nil { return nil, err } - applicationsEndpointUrl := fmt.Sprintf("%s/Applications/v1/", iasTenantUrl) - apiClient, err := api.NewClientWithResponses(applicationsEndpointUrl, api.WithRequestEditorFn(basicAuthProvider.Intercept)) + applicationsEndpointURL := fmt.Sprintf("%s/Applications/v1/", iasTenantUrl) + apiClient, err := api.NewClientWithResponses(applicationsEndpointURL, api.WithRequestEditorFn(basicAuthProvider.Intercept)) if err != nil { return nil, err } const timeout = time.Second * 5 - oidcHttpClient := &http.Client{ + oidcHTTPClient := &http.Client{ Timeout: timeout, } return &client{ api: apiClient, - oidcClient: oidc.NewOidcClient(oidcHttpClient, iasTenantUrl), + oidcClient: oidc.NewOidcClient(oidcHTTPClient, iasTenantUrl), credentials: &Credentials{URL: iasTenantUrl, Username: user, Password: password}, }, nil } @@ -51,7 +61,7 @@ type client struct { oidcClient oidc.Client // The token URL of the IAS client. Since this URL should only change when the tenant changes and this will lead to the initialization of // a new client, we can cache the URL to avoid an additional request at each application creation. - tokenUrl *string + tokenURL *string // The jwks URI of the IAS client. Since this URI should only change when the tenant changes and this will lead to the initialization of // a new client, we can cache the URI to avoid an additional request at each application creation. jwksURI *string @@ -82,28 +92,28 @@ func (c *client) CreateApplication(ctx context.Context, name string) (Applicatio } if res.StatusCode() != http.StatusOK { kcontrollerruntime.Log.Error(err, "Failed to delete existing application", "id", *existingApp.Id, "statusCode", res.StatusCode()) - return Application{}, errors.New("failed to delete existing application before creation") + return Application{}, errDeleteExistingApplicationBeforeCreation } } - appId, err := c.createNewApplication(ctx, name) + appID, err := c.createNewApplication(ctx, name) if err != nil { return Application{}, err } - kcontrollerruntime.Log.Info("Created application", "name", name, "id", appId) + kcontrollerruntime.Log.Info("Created application", "name", name, "id", appID) - clientSecret, err := c.createSecret(ctx, appId) + clientSecret, err := c.createSecret(ctx, appID) if err != nil { return Application{}, err } - clientId, err := c.getClientId(ctx, appId) + clientID, err := c.getClientID(ctx, appID) if err != nil { return Application{}, err } // Since the token url is not part of the application response, we have to fetch it from the OIDC configuration. - tokenUrl, err := c.GetTokenUrl(ctx) + tokenURL, err := c.GetTokenURL(ctx) if err != nil { return Application{}, err } @@ -114,23 +124,23 @@ func (c *client) CreateApplication(ctx context.Context, name string) (Applicatio return Application{}, err } - return NewApplication(appId.String(), *clientId, *clientSecret, *tokenUrl, *jwksURI), nil + return NewApplication(appID.String(), *clientID, *clientSecret, *tokenURL, *jwksURI), nil } -func (c *client) GetTokenUrl(ctx context.Context) (*string, error) { - if c.tokenUrl == nil { +func (c *client) GetTokenURL(ctx context.Context) (*string, error) { + if c.tokenURL == nil { tokenEndpoint, err := c.oidcClient.GetTokenEndpoint(ctx) if err != nil { return nil, err } if tokenEndpoint == nil { - return nil, errors.New("failed to fetch token url") + return nil, errFetchTokenURL } - c.tokenUrl = tokenEndpoint + c.tokenURL = tokenEndpoint } - return c.tokenUrl, nil + return c.tokenURL, nil } func (c *client) GetJWKSURI(ctx context.Context) (*string, error) { @@ -140,7 +150,7 @@ func (c *client) GetJWKSURI(ctx context.Context) (*string, error) { return nil, err } if jwksURI == nil { - return nil, errors.New("failed to fetch jwks uri") + return nil, errFetchJWKSURI } c.jwksURI = jwksURI @@ -176,7 +186,7 @@ func (c *client) getApplicationByName(ctx context.Context, name string) (*api.Ap if res.StatusCode() != http.StatusOK { kcontrollerruntime.Log.Error(err, "Failed to fetch existing applications filtered by name", "name", name, "statusCode", res.StatusCode()) - return nil, errors.New("failed to fetch existing applications") + return nil, errFetchExistingApplications } if res.JSON200.Applications != nil { @@ -188,7 +198,7 @@ func (c *client) getApplicationByName(ctx context.Context, name string) (*api.Ap case 1: return &(*res.JSON200.Applications)[0], nil default: - return nil, fmt.Errorf("found multiple applications with the same name %s", name) + return nil, errors.Errorf("found multiple applications with the same name %s", name) } } return nil, nil //nolint:nilnil @@ -203,36 +213,36 @@ func (c *client) createNewApplication(ctx context.Context, name string) (uuid.UU if res.StatusCode() != http.StatusCreated { kcontrollerruntime.Log.Error(err, "Failed to create application", "name", name, "statusCode", res.StatusCode()) - return uuid.UUID{}, errors.New("failed to create application") + return uuid.UUID{}, errCreateApplication } - return extractApplicationId(res) + return extractApplicationID(res) } -func (c *client) createSecret(ctx context.Context, appId uuid.UUID) (*string, error) { - res, err := c.api.CreateApiSecretWithResponse(ctx, appId, newSecretRequest()) +func (c *client) createSecret(ctx context.Context, appID uuid.UUID) (*string, error) { + res, err := c.api.CreateApiSecretWithResponse(ctx, appID, newSecretRequest()) if err != nil { return nil, err } if res.StatusCode() != http.StatusCreated { - kcontrollerruntime.Log.Error(err, "Failed to create api secret", "id", appId, "statusCode", res.StatusCode()) - return nil, errors.New("failed to create api secret") + kcontrollerruntime.Log.Error(err, "Failed to create api secret", "id", appID, "statusCode", res.StatusCode()) + return nil, errCreateAPISecret } return res.JSON201.Secret, nil } -func (c *client) getClientId(ctx context.Context, appId uuid.UUID) (*string, error) { +func (c *client) getClientID(ctx context.Context, appID uuid.UUID) (*string, error) { // The client ID is generated only after an API secret is created, so we need to retrieve the application again to get the client ID. - applicationResponse, err := c.api.GetApplicationWithResponse(ctx, appId, &api.GetApplicationParams{}) + applicationResponse, err := c.api.GetApplicationWithResponse(ctx, appID, &api.GetApplicationParams{}) if err != nil { return nil, err } if applicationResponse.StatusCode() != http.StatusOK { - kcontrollerruntime.Log.Error(err, "Failed to retrieve client ID", "id", appId, "statusCode", applicationResponse.StatusCode()) - return nil, errors.New("failed to retrieve client ID") + kcontrollerruntime.Log.Error(err, "Failed to retrieve client ID", "id", appID, "statusCode", applicationResponse.StatusCode()) + return nil, errRetrieveClientID } return applicationResponse.JSON200.UrnSapIdentityApplicationSchemasExtensionSci10Authentication.ClientId, nil } @@ -250,23 +260,23 @@ func (c *client) deleteApplication(ctx context.Context, id uuid.UUID) error { if res.StatusCode() != http.StatusOK { kcontrollerruntime.Log.Error(err, "Failed to delete application", "id", id, "statusCode", res.StatusCode()) - return errors.New("failed to delete application") + return errDeleteApplication } return nil } -func extractApplicationId(createAppResponse *api.CreateApplicationResponse) (uuid.UUID, error) { +func extractApplicationID(createAppResponse *api.CreateApplicationResponse) (uuid.UUID, error) { // The application ID is only returned as the last part in the location header locationHeader := createAppResponse.HTTPResponse.Header.Get("Location") s := strings.Split(locationHeader, "/") - appId := s[len(s)-1] + appID := s[len(s)-1] - parsedAppId, err := uuid.Parse(appId) + parsedAppID, err := uuid.Parse(appID) if err != nil { - return parsedAppId, errors.Wrap(err, "failed to retrieve application ID from header") + return parsedAppID, errors.Wrap(err, "failed to retrieve application ID from header") } - return parsedAppId, nil + return parsedAppID, nil } func newIasApplication(name string) api.Application { diff --git a/internal/ias/client_test.go b/internal/ias/client_test.go index 7bdbac9..65401a4 100644 --- a/internal/ias/client_test.go +++ b/internal/ias/client_test.go @@ -17,12 +17,12 @@ import ( ) func Test_CreateApplication(t *testing.T) { - appId := uuid.MustParse("90764f89-f041-4ccf-8da9-7a7c2d60d7fc") + appID := uuid.MustParse("90764f89-f041-4ccf-8da9-7a7c2d60d7fc") tests := []struct { name string - givenApiMock func() *mocks.ClientWithResponsesInterface + givenAPIMock func() *mocks.ClientWithResponsesInterface oidcClientMock *eamoidcmocks.Client - clientTokenUrlMock *string + clientTokenURLMock *string clientJWKSURIMock *string assertCalls func(*testing.T, *mocks.ClientWithResponsesInterface) wantApp Application @@ -30,13 +30,13 @@ func Test_CreateApplication(t *testing.T) { }{ { name: "should create new application when fetching existing applications returns status 200 and no applications", - givenApiMock: func() *mocks.ClientWithResponsesInterface { + givenAPIMock: func() *mocks.ClientWithResponsesInterface { clientMock := mocks.ClientWithResponsesInterface{} mockGetAllApplicationsWithResponseStatusOkEmptyResponse(&clientMock) - mockCreateApplicationWithResponseStatusCreated(&clientMock, appId.String()) - mockCreateApiSecretWithResponseStatusCreated(&clientMock, appId) - mockGetApplicationWithResponseStatusOK(&clientMock, appId) + mockCreateApplicationWithResponseStatusCreated(&clientMock, appID.String()) + mockCreateAPISecretWithResponseStatusCreated(&clientMock, appID) + mockGetApplicationWithResponseStatusOK(&clientMock, appID) return &clientMock }, @@ -46,7 +46,7 @@ func Test_CreateApplication(t *testing.T) { ptr.To("https://test.com/certs"), ), wantApp: NewApplication( - appId.String(), + appID.String(), "clientIdMock", "clientSecretMock", "https://test.com/token", @@ -55,14 +55,14 @@ func Test_CreateApplication(t *testing.T) { }, { name: "should create new application when fetching existing applications returns status 404", - givenApiMock: func() *mocks.ClientWithResponsesInterface { + givenAPIMock: func() *mocks.ClientWithResponsesInterface { clientMock := mocks.ClientWithResponsesInterface{} - appId := uuid.MustParse("90764f89-f041-4ccf-8da9-7a7c2d60d7fc") + appID := uuid.MustParse("90764f89-f041-4ccf-8da9-7a7c2d60d7fc") mockGetAllApplicationsWithResponseStatusNotFound(&clientMock) - mockCreateApplicationWithResponseStatusCreated(&clientMock, appId.String()) - mockCreateApiSecretWithResponseStatusCreated(&clientMock, appId) - mockGetApplicationWithResponseStatusOK(&clientMock, appId) + mockCreateApplicationWithResponseStatusCreated(&clientMock, appID.String()) + mockCreateAPISecretWithResponseStatusCreated(&clientMock, appID) + mockGetApplicationWithResponseStatusOK(&clientMock, appID) return &clientMock }, @@ -72,7 +72,7 @@ func Test_CreateApplication(t *testing.T) { ptr.To("https://test.com/certs"), ), wantApp: NewApplication( - appId.String(), + appID.String(), "clientIdMock", "clientSecretMock", "https://test.com/token", @@ -81,17 +81,17 @@ func Test_CreateApplication(t *testing.T) { }, { name: "should recreate application when application already exists", - givenApiMock: func() *mocks.ClientWithResponsesInterface { + givenAPIMock: func() *mocks.ClientWithResponsesInterface { clientMock := mocks.ClientWithResponsesInterface{} - existingAppId := uuid.MustParse("5ab797c0-80a0-4ca4-ad7f-50a0f40231d6") - mockGetAllApplicationsWithResponseStatusOk(&clientMock, existingAppId) - mockDeleteApplicationWithResponseStatusOk(&clientMock, existingAppId) + existingAppID := uuid.MustParse("5ab797c0-80a0-4ca4-ad7f-50a0f40231d6") + mockGetAllApplicationsWithResponseStatusOk(&clientMock, existingAppID) + mockDeleteApplicationWithResponseStatusOk(&clientMock, existingAppID) - newAppId := uuid.MustParse("90764f89-f041-4ccf-8da9-7a7c2d60d7fc") - mockCreateApplicationWithResponseStatusCreated(&clientMock, newAppId.String()) - mockCreateApiSecretWithResponseStatusCreated(&clientMock, newAppId) - mockGetApplicationWithResponseStatusOK(&clientMock, newAppId) + newAppID := uuid.MustParse("90764f89-f041-4ccf-8da9-7a7c2d60d7fc") + mockCreateApplicationWithResponseStatusCreated(&clientMock, newAppID.String()) + mockCreateAPISecretWithResponseStatusCreated(&clientMock, newAppID) + mockGetApplicationWithResponseStatusOK(&clientMock, newAppID) return &clientMock }, @@ -101,7 +101,7 @@ func Test_CreateApplication(t *testing.T) { ptr.To("https://test.com/certs"), ), wantApp: NewApplication( - appId.String(), + appID.String(), "clientIdMock", "clientSecretMock", "https://test.com/token", @@ -110,20 +110,20 @@ func Test_CreateApplication(t *testing.T) { }, { name: "should return an error when multiple applications exist for the given name", - givenApiMock: func() *mocks.ClientWithResponsesInterface { + givenAPIMock: func() *mocks.ClientWithResponsesInterface { clientMock := mocks.ClientWithResponsesInterface{} - appId2 := uuid.MustParse("41de6fec-e0fc-47d7-b35c-3b19c4927e4f") - mockGetAllApplicationsWithResponseStatusOk(&clientMock, appId, appId2) + appID2 := uuid.MustParse("41de6fec-e0fc-47d7-b35c-3b19c4927e4f") + mockGetAllApplicationsWithResponseStatusOk(&clientMock, appID, appID2) return &clientMock }, wantApp: Application{}, - wantError: errors.New("found multiple applications with the same name Test-App-Name"), + wantError: errors.New("found multiple applications with the same name Test-App-Name"), //nolint:goerr113 // used one time only in tests. }, { name: "should return error when application ID can't be retrieved from location header", - givenApiMock: func() *mocks.ClientWithResponsesInterface { + givenAPIMock: func() *mocks.ClientWithResponsesInterface { clientMock := mocks.ClientWithResponsesInterface{} mockGetAllApplicationsWithResponseStatusOkEmptyResponse(&clientMock) mockCreateApplicationWithResponseStatusCreated(&clientMock, "non-uuid-application-id") @@ -131,36 +131,36 @@ func Test_CreateApplication(t *testing.T) { return &clientMock }, wantApp: Application{}, - wantError: errors.New("failed to retrieve application ID from header: invalid UUID length: 23"), + wantError: errors.New("failed to retrieve application ID from header: invalid UUID length: 23"), //nolint:goerr113 // used one time only in tests. }, { name: "should return error when fetching existing application failed", - givenApiMock: func() *mocks.ClientWithResponsesInterface { + givenAPIMock: func() *mocks.ClientWithResponsesInterface { clientMock := mocks.ClientWithResponsesInterface{} mockGetAllApplicationsWithResponseStatusInternalServerError(&clientMock) return &clientMock }, wantApp: Application{}, - wantError: errors.New("failed to fetch existing applications"), + wantError: errFetchExistingApplications, }, { name: "should return error when application exists and deletion failed", - givenApiMock: func() *mocks.ClientWithResponsesInterface { + givenAPIMock: func() *mocks.ClientWithResponsesInterface { clientMock := mocks.ClientWithResponsesInterface{} - appId := uuid.MustParse("90764f89-f041-4ccf-8da9-7a7c2d60d7fc") - mockGetAllApplicationsWithResponseStatusOk(&clientMock, appId) + appID := uuid.MustParse("90764f89-f041-4ccf-8da9-7a7c2d60d7fc") + mockGetAllApplicationsWithResponseStatusOk(&clientMock, appID) mockDeleteApplicationWithResponseStatusInternalServerError(&clientMock) return &clientMock }, wantApp: Application{}, - wantError: errors.New("failed to delete existing application before creation"), + wantError: errDeleteExistingApplicationBeforeCreation, }, { name: "should return error when application is not created", - givenApiMock: func() *mocks.ClientWithResponsesInterface { + givenAPIMock: func() *mocks.ClientWithResponsesInterface { clientMock := mocks.ClientWithResponsesInterface{} mockGetAllApplicationsWithResponseStatusOkEmptyResponse(&clientMock) @@ -169,88 +169,88 @@ func Test_CreateApplication(t *testing.T) { return &clientMock }, wantApp: Application{}, - wantError: errors.New("failed to create application"), + wantError: errCreateApplication, }, { name: "should return error when secret is not created", - givenApiMock: func() *mocks.ClientWithResponsesInterface { + givenAPIMock: func() *mocks.ClientWithResponsesInterface { clientMock := mocks.ClientWithResponsesInterface{} mockGetAllApplicationsWithResponseStatusOkEmptyResponse(&clientMock) - appId := uuid.MustParse("90764f89-f041-4ccf-8da9-7a7c2d60d7fc") - mockCreateApplicationWithResponseStatusCreated(&clientMock, appId.String()) - mockCreateApiSecretWithResponseStatusInternalServerError(&clientMock) + appID := uuid.MustParse("90764f89-f041-4ccf-8da9-7a7c2d60d7fc") + mockCreateApplicationWithResponseStatusCreated(&clientMock, appID.String()) + mockCreateAPISecretWithResponseStatusInternalServerError(&clientMock) return &clientMock }, wantApp: Application{}, - wantError: errors.New("failed to create api secret"), + wantError: errCreateAPISecret, }, { name: "should return error when client id wasn't fetched", - givenApiMock: func() *mocks.ClientWithResponsesInterface { + givenAPIMock: func() *mocks.ClientWithResponsesInterface { clientMock := mocks.ClientWithResponsesInterface{} - appId := uuid.MustParse("90764f89-f041-4ccf-8da9-7a7c2d60d7fc") + appID := uuid.MustParse("90764f89-f041-4ccf-8da9-7a7c2d60d7fc") mockGetAllApplicationsWithResponseStatusOkEmptyResponse(&clientMock) - mockCreateApplicationWithResponseStatusCreated(&clientMock, appId.String()) - mockCreateApiSecretWithResponseStatusCreated(&clientMock, appId) + mockCreateApplicationWithResponseStatusCreated(&clientMock, appID.String()) + mockCreateAPISecretWithResponseStatusCreated(&clientMock, appID) mockGetApplicationWithResponseStatusInternalServerError(&clientMock) return &clientMock }, wantApp: Application{}, - wantError: errors.New("failed to retrieve client ID"), + wantError: errRetrieveClientID, }, { name: "should return an error when token URL wasn't fetched", - givenApiMock: func() *mocks.ClientWithResponsesInterface { + givenAPIMock: func() *mocks.ClientWithResponsesInterface { clientMock := mocks.ClientWithResponsesInterface{} mockGetAllApplicationsWithResponseStatusOkEmptyResponse(&clientMock) - mockCreateApplicationWithResponseStatusCreated(&clientMock, appId.String()) - mockCreateApiSecretWithResponseStatusCreated(&clientMock, appId) - mockGetApplicationWithResponseStatusOK(&clientMock, appId) + mockCreateApplicationWithResponseStatusCreated(&clientMock, appID.String()) + mockCreateAPISecretWithResponseStatusCreated(&clientMock, appID) + mockGetApplicationWithResponseStatusOK(&clientMock, appID) return &clientMock }, oidcClientMock: mockGetTokenEndpoint(t, nil), wantApp: Application{}, - wantError: errors.New("failed to fetch token url"), + wantError: errFetchTokenURL, }, { name: "should return an error when jwks URI wasn't fetched", - givenApiMock: func() *mocks.ClientWithResponsesInterface { + givenAPIMock: func() *mocks.ClientWithResponsesInterface { clientMock := mocks.ClientWithResponsesInterface{} mockGetAllApplicationsWithResponseStatusOkEmptyResponse(&clientMock) - mockCreateApplicationWithResponseStatusCreated(&clientMock, appId.String()) - mockCreateApiSecretWithResponseStatusCreated(&clientMock, appId) - mockGetApplicationWithResponseStatusOK(&clientMock, appId) + mockCreateApplicationWithResponseStatusCreated(&clientMock, appID.String()) + mockCreateAPISecretWithResponseStatusCreated(&clientMock, appID) + mockGetApplicationWithResponseStatusOK(&clientMock, appID) return &clientMock }, oidcClientMock: mockClient(t, ptr.To("https://test.com/token"), nil), wantApp: Application{}, - wantError: errors.New("failed to fetch jwks uri"), + wantError: errFetchJWKSURI, }, { name: "should create new application without fetching token URL when it is already cached in the client", - givenApiMock: func() *mocks.ClientWithResponsesInterface { + givenAPIMock: func() *mocks.ClientWithResponsesInterface { clientMock := mocks.ClientWithResponsesInterface{} mockGetAllApplicationsWithResponseStatusOkEmptyResponse(&clientMock) - mockCreateApplicationWithResponseStatusCreated(&clientMock, appId.String()) - mockCreateApiSecretWithResponseStatusCreated(&clientMock, appId) - mockGetApplicationWithResponseStatusOK(&clientMock, appId) + mockCreateApplicationWithResponseStatusCreated(&clientMock, appID.String()) + mockCreateAPISecretWithResponseStatusCreated(&clientMock, appID) + mockGetApplicationWithResponseStatusOK(&clientMock, appID) return &clientMock }, - clientTokenUrlMock: ptr.To("https://from-cache.com/token"), + clientTokenURLMock: ptr.To("https://from-cache.com/token"), clientJWKSURIMock: ptr.To("https://from-cache.com/certs"), wantApp: NewApplication( - appId.String(), + appID.String(), "clientIdMock", "clientSecretMock", "https://from-cache.com/token", @@ -261,13 +261,13 @@ func Test_CreateApplication(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { // given - apiMock := tt.givenApiMock() + apiMock := tt.givenAPIMock() oidcMock := tt.oidcClientMock client := client{ api: apiMock, oidcClient: oidcMock, - tokenUrl: tt.clientTokenUrlMock, + tokenURL: tt.clientTokenURLMock, jwksURI: tt.clientJWKSURIMock, } @@ -300,37 +300,37 @@ func Test_CreateApplication(t *testing.T) { func Test_DeleteApplication(t *testing.T) { tests := []struct { name string - givenApiMock func() *mocks.ClientWithResponsesInterface + givenAPIMock func() *mocks.ClientWithResponsesInterface wantError error }{ { name: "should delete application", - givenApiMock: func() *mocks.ClientWithResponsesInterface { + givenAPIMock: func() *mocks.ClientWithResponsesInterface { clientMock := mocks.ClientWithResponsesInterface{} - appId := uuid.MustParse("90764f89-f041-4ccf-8da9-7a7c2d60d7fc") - mockGetAllApplicationsWithResponseStatusOk(&clientMock, appId) - mockDeleteApplicationWithResponseStatusOk(&clientMock, appId) + appID := uuid.MustParse("90764f89-f041-4ccf-8da9-7a7c2d60d7fc") + mockGetAllApplicationsWithResponseStatusOk(&clientMock, appID) + mockDeleteApplicationWithResponseStatusOk(&clientMock, appID) return &clientMock }, }, { name: "should return error when application is not deleted", - givenApiMock: func() *mocks.ClientWithResponsesInterface { + givenAPIMock: func() *mocks.ClientWithResponsesInterface { clientMock := mocks.ClientWithResponsesInterface{} - appId := uuid.MustParse("90764f89-f041-4ccf-8da9-7a7c2d60d7fc") - mockGetAllApplicationsWithResponseStatusOk(&clientMock, appId) + appID := uuid.MustParse("90764f89-f041-4ccf-8da9-7a7c2d60d7fc") + mockGetAllApplicationsWithResponseStatusOk(&clientMock, appID) mockDeleteApplicationWithResponseStatusInternalServerError(&clientMock) return &clientMock }, - wantError: errors.New("failed to delete application"), + wantError: errDeleteApplication, }, { name: "should not return an error when application doesn't exist", - givenApiMock: func() *mocks.ClientWithResponsesInterface { + givenAPIMock: func() *mocks.ClientWithResponsesInterface { clientMock := mocks.ClientWithResponsesInterface{} mockGetAllApplicationsWithResponseStatusOkEmptyResponse(&clientMock) @@ -340,11 +340,11 @@ func Test_DeleteApplication(t *testing.T) { }, { name: "should not return an error when app was found, but was already deleted", - givenApiMock: func() *mocks.ClientWithResponsesInterface { + givenAPIMock: func() *mocks.ClientWithResponsesInterface { clientMock := mocks.ClientWithResponsesInterface{} - appId := uuid.MustParse("90764f89-f041-4ccf-8da9-7a7c2d60d7fc") - mockGetAllApplicationsWithResponseStatusOk(&clientMock, appId) + appID := uuid.MustParse("90764f89-f041-4ccf-8da9-7a7c2d60d7fc") + mockGetAllApplicationsWithResponseStatusOk(&clientMock, appID) mockDeleteApplicationWithResponseStatusNotFound(&clientMock) return &clientMock @@ -355,7 +355,7 @@ func Test_DeleteApplication(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { // given - apiMock := tt.givenApiMock() + apiMock := tt.givenAPIMock() client := client{ api: apiMock, @@ -409,8 +409,8 @@ func mockGetAllApplicationsWithResponseStatusOkEmptyResponse(clientMock *mocks.C func mockGetAllApplicationsWithResponseStatusOk(clientMock *mocks.ClientWithResponsesInterface, appIds ...uuid.UUID) { appResponses := make([]api.ApplicationResponse, 0) - for _, appId := range appIds { - id := appId + for _, appID := range appIds { + id := appID appResponses = append(appResponses, api.ApplicationResponse{ Id: &id, }) @@ -437,19 +437,19 @@ func mockCreateApplicationWithResponseStatusInternalServerError(clientMock *mock }, nil) } -func mockCreateApplicationWithResponseStatusCreated(clientMock *mocks.ClientWithResponsesInterface, appId string) { +func mockCreateApplicationWithResponseStatusCreated(clientMock *mocks.ClientWithResponsesInterface, appID string) { clientMock.On("CreateApplicationWithResponse", mock.Anything, mock.Anything, newIasApplication("Test-App-Name")). Return(&api.CreateApplicationResponse{ HTTPResponse: &http.Response{ StatusCode: http.StatusCreated, Header: map[string][]string{ - "Location": {fmt.Sprintf("https://test.com/v1/Applications/%s", appId)}, + "Location": {fmt.Sprintf("https://test.com/v1/Applications/%s", appID)}, }, }, }, nil) } -func mockCreateApiSecretWithResponseStatusInternalServerError(clientMock *mocks.ClientWithResponsesInterface) { +func mockCreateAPISecretWithResponseStatusInternalServerError(clientMock *mocks.ClientWithResponsesInterface) { clientMock.On("CreateApiSecretWithResponse", mock.Anything, mock.Anything, mock.Anything). Return(&api.CreateApiSecretResponse{ HTTPResponse: &http.Response{ @@ -458,14 +458,14 @@ func mockCreateApiSecretWithResponseStatusInternalServerError(clientMock *mocks. }, nil) } -func mockCreateApiSecretWithResponseStatusCreated(clientMock *mocks.ClientWithResponsesInterface, appId uuid.UUID) { +func mockCreateAPISecretWithResponseStatusCreated(clientMock *mocks.ClientWithResponsesInterface, appID uuid.UUID) { d := "eventing-auth-manager" s := "clientSecretMock" secretRequest := api.CreateApiSecretJSONRequestBody{ AuthorizationScopes: &[]api.AuthorizationScope{"oAuth"}, Description: &d, } - clientMock.On("CreateApiSecretWithResponse", mock.Anything, appId, secretRequest). + clientMock.On("CreateApiSecretWithResponse", mock.Anything, appID, secretRequest). Return(&api.CreateApiSecretResponse{ HTTPResponse: &http.Response{ StatusCode: http.StatusCreated, @@ -485,9 +485,9 @@ func mockGetApplicationWithResponseStatusInternalServerError(clientMock *mocks.C }, nil) } -func mockGetApplicationWithResponseStatusOK(clientMock *mocks.ClientWithResponsesInterface, appId uuid.UUID) { +func mockGetApplicationWithResponseStatusOK(clientMock *mocks.ClientWithResponsesInterface, appID uuid.UUID) { cID := "clientIdMock" - clientMock.On("GetApplicationWithResponse", mock.Anything, appId, mock.Anything). + clientMock.On("GetApplicationWithResponse", mock.Anything, appID, mock.Anything). Return(&api.GetApplicationResponse{ HTTPResponse: &http.Response{ StatusCode: http.StatusOK, @@ -500,8 +500,8 @@ func mockGetApplicationWithResponseStatusOK(clientMock *mocks.ClientWithResponse }, nil) } -func mockDeleteApplicationWithResponseStatusOk(clientMock *mocks.ClientWithResponsesInterface, appId uuid.UUID) { - clientMock.On("DeleteApplicationWithResponse", mock.Anything, appId). +func mockDeleteApplicationWithResponseStatusOk(clientMock *mocks.ClientWithResponsesInterface, appID uuid.UUID) { + clientMock.On("DeleteApplicationWithResponse", mock.Anything, appID). Return(&api.DeleteApplicationResponse{ HTTPResponse: &http.Response{ StatusCode: http.StatusOK, @@ -527,17 +527,17 @@ func mockDeleteApplicationWithResponseStatusNotFound(clientMock *mocks.ClientWit }, nil) } -func mockClient(t *testing.T, tokenUrl, jwksURI *string) *eamoidcmocks.Client { +func mockClient(t *testing.T, tokenURL, jwksURI *string) *eamoidcmocks.Client { t.Helper() clientMock := eamoidcmocks.NewClient(t) - clientMock.On("GetTokenEndpoint", mock.Anything).Return(tokenUrl, nil) + clientMock.On("GetTokenEndpoint", mock.Anything).Return(tokenURL, nil) clientMock.On("GetJWKSURI", mock.Anything).Return(jwksURI, nil) return clientMock } -func mockGetTokenEndpoint(t *testing.T, tokenUrl *string) *eamoidcmocks.Client { +func mockGetTokenEndpoint(t *testing.T, tokenURL *string) *eamoidcmocks.Client { t.Helper() clientMock := eamoidcmocks.NewClient(t) - clientMock.On("GetTokenEndpoint", mock.Anything).Return(tokenUrl, nil) + clientMock.On("GetTokenEndpoint", mock.Anything).Return(tokenURL, nil) return clientMock } diff --git a/internal/ias/ias_config.go b/internal/ias/ias_config.go index 25b740c..4b15f92 100644 --- a/internal/ias/ias_config.go +++ b/internal/ias/ias_config.go @@ -2,8 +2,8 @@ package ias import ( "context" - "fmt" + "github.com/pkg/errors" kcorev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" kpkgclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -30,39 +30,38 @@ type Credentials struct { } // ReadCredentials fetches ias credentials from secret in the cluster. Reads from env vars if secret is missing. -var ReadCredentials = func(namespace, name string, k8sClient kpkgclient.Client) (*Credentials, error) { +var ReadCredentials = func(namespace, name string, k8sClient kpkgclient.Client) (*Credentials, error) { //nolint:gochecknoglobals // For mocking purposes. namespacedName := types.NamespacedName{ Namespace: namespace, Name: name, } iasSecret := &kcorev1.Secret{} - err := k8sClient.Get(context.TODO(), namespacedName, iasSecret) - if err != nil { + if err := k8sClient.Get(context.TODO(), namespacedName, iasSecret); err != nil { return nil, err } var exists bool var url, username, password []byte - var errors error + var err error if url, exists = iasSecret.Data[urlString]; !exists { - errors = fmt.Errorf("key %s is not found in ias secret", urlString) + err = errors.Errorf("key %s is not found in ias secret", urlString) } if username, exists = iasSecret.Data[usernameString]; !exists { - if errors != nil { - errors = fmt.Errorf("%w: key %s is not found in ias secret", errors, usernameString) + if err != nil { + err = errors.Wrapf(err, "key %s is not found in ias secret", usernameString) } else { - errors = fmt.Errorf("key %s is not found in ias secret", usernameString) + err = errors.Errorf("key %s is not found in ias secret", usernameString) } } if password, exists = iasSecret.Data[passwordString]; !exists { - if errors != nil { - errors = fmt.Errorf("%w: key %s is not found in ias secret", errors, passwordString) + if err != nil { + err = errors.Wrapf(err, "key %s is not found in ias secret", passwordString) } else { - errors = fmt.Errorf("key %s is not found in ias secret", passwordString) + err = errors.Errorf("key %s is not found in ias secret", passwordString) } } - if errors != nil { - return nil, errors + if err != nil { + return nil, err } iasConfig := NewCredentials(string(url), string(username), string(password)) return iasConfig, nil diff --git a/internal/ias/ias_config_test.go b/internal/ias/ias_config_test.go index 48ef5e9..81b6c15 100644 --- a/internal/ias/ias_config_test.go +++ b/internal/ias/ias_config_test.go @@ -1,11 +1,10 @@ package ias import ( - "errors" - "fmt" "testing" "github.com/kyma-project/eventing-auth-manager/internal/ias/internal/mocks" + "github.com/pkg/errors" "github.com/stretchr/testify/require" kcorev1 "k8s.io/api/core/v1" kmetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -15,7 +14,7 @@ import ( func Test_ReadCredentials(t *testing.T) { testNamespace := "mock-namespace" testName := "mock-name" - testUrl := "https://test.url.com" + testURL := "https://test.url.com" testUsername := "test-username" testPassword := "test-password" @@ -32,10 +31,10 @@ func Test_ReadCredentials(t *testing.T) { MockFunction: func() error { return nil }, - MockSecret: createMockSecret(testNamespace, testName, testUrl, testUsername, testPassword), + MockSecret: createMockSecret(testNamespace, testName, testURL, testUsername, testPassword), }, wantCredentials: Credentials{ - URL: testUrl, + URL: testURL, Username: testUsername, Password: testPassword, }, @@ -46,7 +45,7 @@ func Test_ReadCredentials(t *testing.T) { MockFunction: func() error { return errors.New("mock error") }, - MockSecret: createMockSecret(testNamespace, testName, testUrl, testUsername, testPassword), + MockSecret: createMockSecret(testNamespace, testName, testURL, testUsername, testPassword), }, wantError: errors.New("mock error"), }, @@ -58,8 +57,8 @@ func Test_ReadCredentials(t *testing.T) { }, MockSecret: createMockSecretWithoutDataFields(testNamespace, testPassword), }, - wantError: fmt.Errorf("key %s is not found in ias secret: key %s is not found in ias secret: key %s is not found in ias secret", - urlString, usernameString, passwordString), + wantError: errors.Errorf("key %s is not found in ias secret: key %s is not found in ias secret: key %s is not found in ias secret", + passwordString, usernameString, urlString), }, { name: "Fails with missing username data fields error", @@ -67,9 +66,9 @@ func Test_ReadCredentials(t *testing.T) { MockFunction: func() error { return nil }, - MockSecret: createMockSecretWithoutUsernameDataFields(testNamespace, testName, testUrl, testPassword), + MockSecret: createMockSecretWithoutUsernameDataFields(testNamespace, testName, testURL, testPassword), }, - wantError: fmt.Errorf("key %s is not found in ias secret", usernameString), + wantError: errors.Errorf("key %s is not found in ias secret", usernameString), }, { name: "Fails with missing username and password data fields error", @@ -77,9 +76,9 @@ func Test_ReadCredentials(t *testing.T) { MockFunction: func() error { return nil }, - MockSecret: createMockSecretWithoutUsernameAndPasswordDataFields(testNamespace, testName, testUrl), + MockSecret: createMockSecretWithoutUsernameAndPasswordDataFields(testNamespace, testName, testURL), }, - wantError: fmt.Errorf("key %s is not found in ias secret: key %s is not found in ias secret", usernameString, passwordString), + wantError: errors.Errorf("key %s is not found in ias secret: key %s is not found in ias secret", passwordString, usernameString), }, } @@ -98,7 +97,7 @@ func Test_ReadCredentials(t *testing.T) { } } -func createMockSecret(testNamespace, testName, testUrl, testUsername, testPassword string) *kcorev1.Secret { +func createMockSecret(testNamespace, testName, testURL, testUsername, testPassword string) *kcorev1.Secret { s := &kcorev1.Secret{ ObjectMeta: kmetav1.ObjectMeta{ Name: testNamespace, @@ -106,7 +105,7 @@ func createMockSecret(testNamespace, testName, testUrl, testUsername, testPasswo }, Data: map[string][]byte{}, } - s.Data[urlString] = []byte(testUrl) + s.Data[urlString] = []byte(testURL) s.Data[usernameString] = []byte(testUsername) s.Data[passwordString] = []byte(testPassword) return s @@ -124,7 +123,7 @@ func createMockSecretWithoutDataFields(testNamespace, testName string) *kcorev1. return s } -func createMockSecretWithoutUsernameDataFields(testNamespace, testName, testUrl, testPassword string) *kcorev1.Secret { +func createMockSecretWithoutUsernameDataFields(testNamespace, testName, testURL, testPassword string) *kcorev1.Secret { s := &kcorev1.Secret{ ObjectMeta: kmetav1.ObjectMeta{ Name: testNamespace, @@ -132,12 +131,12 @@ func createMockSecretWithoutUsernameDataFields(testNamespace, testName, testUrl, }, Data: map[string][]byte{}, } - s.Data[urlString] = []byte(testUrl) + s.Data[urlString] = []byte(testURL) s.Data[passwordString] = []byte(testPassword) return s } -func createMockSecretWithoutUsernameAndPasswordDataFields(testNamespace, testName, testUrl string) *kcorev1.Secret { +func createMockSecretWithoutUsernameAndPasswordDataFields(testNamespace, testName, testURL string) *kcorev1.Secret { s := &kcorev1.Secret{ ObjectMeta: kmetav1.ObjectMeta{ Name: testNamespace, @@ -145,6 +144,6 @@ func createMockSecretWithoutUsernameAndPasswordDataFields(testNamespace, testNam }, Data: map[string][]byte{}, } - s.Data[urlString] = []byte(testUrl) + s.Data[urlString] = []byte(testURL) return s } diff --git a/internal/ias/internal/api/SCI_Application_Directory.yaml b/internal/ias/internal/api/SCI_Application_Directory.yaml index 7ca1dd1..d808365 100644 --- a/internal/ias/internal/api/SCI_Application_Directory.yaml +++ b/internal/ias/internal/api/SCI_Application_Directory.yaml @@ -450,11 +450,11 @@ paths: * name ### Authentication Schema - * clientId + * clientID required: false examples: - clientId: - value: clientId eq b92c95c0-42ea-45c0-9cbb-aa49c1020e42 + clientID: + value: clientID eq b92c95c0-42ea-45c0-9cbb-aa49c1020e42 name: value: name eq SFSF parentApplicationId: @@ -806,11 +806,11 @@ components: maxLength: 255 pattern: ^[a-zA-Z0-9_]+$ example: global - clientId: + clientID: type: string maxLength: 36 example: eac15e81-6703-4c67-8719-ae5c6c865ad3 - description: it's the same for clientId in OIDC flows + description: it's the same for clientID in OIDC flows apiCertificates: type: array maxItems: 20 @@ -898,8 +898,8 @@ components: $ref: '#/components/schemas/ConsumedService' example: - serviceInstanceId: service-instance-id - appId: app-id - clientId: client-id + appID: app-id + clientID: client-id planName: plan-name inherit: true providedApis: @@ -916,7 +916,7 @@ components: items: $ref: '#/components/schemas/ConsumedApi' example: - - appId: app-id + - appID: app-id apiName: read name: S4 riskBasedAuthentication: @@ -1384,17 +1384,17 @@ components: ConsumedService: type: object required: - - appId - - clientId + - appID + - clientID - inherit properties: serviceInstanceId: type: string maxLength: 255 - appId: + appID: type: string format: uuid - clientId: + clientID: type: string maxLength: 255 planName: @@ -1414,14 +1414,14 @@ components: ConsumedApi: type: object required: - - appId + - appID - apiName - name properties: - appId: + appID: type: string format: uuid - clientId: + clientID: type: string maxLength: 255 apiName: @@ -1679,4 +1679,4 @@ components: default: charged version: type: string - example: 1.0 \ No newline at end of file + example: 1.0 diff --git a/internal/ias/internal/api/ias.gen.go b/internal/ias/internal/api/ias.gen.go index f29c0cf..074bd04 100644 --- a/internal/ias/internal/api/ias.gen.go +++ b/internal/ias/internal/api/ias.gen.go @@ -340,8 +340,8 @@ type AuthenticationSchema struct { BiometricAuthenticationEnabled *bool `json:"biometricAuthenticationEnabled,omitempty"` CaptchaConfig *CaptchaConfig `json:"captchaConfig,omitempty"` - // ClientId it's the same for clientId in OIDC flows - ClientId *string `json:"clientId,omitempty"` + // ClientId it's the same for clientID in OIDC flows + ClientId *string `json:"clientID,omitempty"` CompanyId *string `json:"companyId,omitempty"` ConcurrentAccess *AuthenticationSchemaConcurrentAccess `json:"concurrentAccess,omitempty"` ConditionalAuthentication *[]AuthenticationRule `json:"conditionalAuthentication,omitempty"` @@ -440,15 +440,15 @@ type CaptchaConfig struct { // ConsumedApi defines model for ConsumedApi. type ConsumedApi struct { ApiName string `json:"apiName"` - AppId openapi_types.UUID `json:"appId"` - ClientId *string `json:"clientId,omitempty"` + AppId openapi_types.UUID `json:"appID"` + ClientId *string `json:"clientID,omitempty"` Name string `json:"name"` } // ConsumedService defines model for ConsumedService. type ConsumedService struct { - AppId openapi_types.UUID `json:"appId"` - ClientId string `json:"clientId"` + AppId openapi_types.UUID `json:"appID"` + ClientId string `json:"clientID"` Inherit bool `json:"inherit"` PlanName *string `json:"planName,omitempty"` ServiceInstanceId *string `json:"serviceInstanceId,omitempty"` @@ -708,7 +708,7 @@ type GetAllApplicationsParams struct { // * name // // ### Authentication Schema - // * clientId + // * clientID Filter *string `form:"filter,omitempty" json:"filter,omitempty"` // Limit Number of items to return. diff --git a/internal/ias/internal/oidc/oidc-client.go b/internal/ias/internal/oidc/oidc-client.go index 60348bc..cdd33ac 100644 --- a/internal/ias/internal/oidc/oidc-client.go +++ b/internal/ias/internal/oidc/oidc-client.go @@ -6,6 +6,8 @@ import ( "fmt" "io" "net/http" + + "github.com/pkg/errors" ) //go:generate mockery --name=Client --outpkg=mocks --case=underscore @@ -20,14 +22,14 @@ type wellKnown struct { } type client struct { - domainUrl string + domainURL string httpClient *http.Client } // NewOidcClient returns a new OIDC client. The domain URL is used to get the OIDC configuration for a specific tenant, e.g. 'https://some-tenant.accounts400.ondemand.com'. -func NewOidcClient(h *http.Client, domainUrl string) Client { +func NewOidcClient(h *http.Client, domainURL string) Client { return client{ - domainUrl: domainUrl, + domainURL: domainURL, httpClient: h, } } @@ -53,7 +55,7 @@ func (c client) GetJWKSURI(ctx context.Context) (*string, error) { } func (c client) getWellKnown(ctx context.Context) (wellKnown, error) { - url := fmt.Sprintf("%s/.well-known/openid-configuration", c.domainUrl) + url := fmt.Sprintf("%s/.well-known/openid-configuration", c.domainURL) req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { return wellKnown{}, err @@ -79,7 +81,7 @@ func (c client) do(req *http.Request) ([]byte, error) { } if res.StatusCode != http.StatusOK { - return nil, fmt.Errorf("unexpected status code %d", res.StatusCode) + return nil, errors.Errorf("unexpected status code %d", res.StatusCode) } if res.Body != nil { diff --git a/internal/ias/internal/oidc/oidc-client_test.go b/internal/ias/internal/oidc/oidc-client_test.go index a053210..03091ba 100644 --- a/internal/ias/internal/oidc/oidc-client_test.go +++ b/internal/ias/internal/oidc/oidc-client_test.go @@ -29,7 +29,7 @@ func Test_oidcClient_getTokenUrl(t *testing.T) { { name: "should return token endpoint", fields: fields{ - httpClient: mockHttpClientResponseOk([]byte(oidcConfigMock)), + httpClient: mockHTTPClientResponseOk([]byte(oidcConfigMock)), }, want: ptr.To("https://domain-url.com/token"), }, @@ -52,7 +52,7 @@ func Test_oidcClient_getTokenUrl(t *testing.T) { { name: "should return nil when well known contains no token endpoint", fields: fields{ - httpClient: mockHttpClientResponseOk([]byte("{}")), + httpClient: mockHTTPClientResponseOk([]byte("{}")), }, }, { @@ -64,21 +64,21 @@ func Test_oidcClient_getTokenUrl(t *testing.T) { }, nil }), }, - wantErr: errors.New("unexpected status code 500"), + wantErr: errors.New("unexpected status code 500"), //nolint:goerr113 // used one time only in tests. }, { name: "should return error when response body is nil", fields: fields{ - httpClient: mockHttpClientResponseOk(nil), + httpClient: mockHTTPClientResponseOk(nil), }, - wantErr: errors.New("unexpected end of JSON input"), + wantErr: errors.New("unexpected end of JSON input"), //nolint:goerr113 // used one time only in tests. }, { name: "should return error when response body is no json", fields: fields{ - httpClient: mockHttpClientResponseOk([]byte("invalid json")), + httpClient: mockHTTPClientResponseOk([]byte("invalid json")), }, - wantErr: errors.New("invalid character 'i' looking for beginning of value"), + wantErr: errors.New("invalid character 'i' looking for beginning of value"), //nolint:goerr113 // used one time only in tests. }, } for _, tt := range tests { @@ -102,7 +102,7 @@ func Test_oidcClient_getTokenUrl(t *testing.T) { } } -func mockHttpClientResponseOk(body []byte) *http.Client { +func mockHTTPClientResponseOk(body []byte) *http.Client { return fake.CreateHTTPClient(func(request *http.Request) (*http.Response, error) { return &http.Response{ StatusCode: http.StatusOK, diff --git a/internal/ias/types.go b/internal/ias/types.go index d084712..37399bc 100644 --- a/internal/ias/types.go +++ b/internal/ias/types.go @@ -7,19 +7,19 @@ import ( type Application struct { id string - clientId string + clientID string clientSecret string - tokenUrl string - certsUrl string + tokenURL string + certsURL string } -func NewApplication(id, clientId, clientSecret, tokenUrl, certsUrl string) Application { +func NewApplication(id, clientID, clientSecret, tokenURL, certsURL string) Application { return Application{ id: id, - clientId: clientId, + clientID: clientID, clientSecret: clientSecret, - tokenUrl: tokenUrl, - certsUrl: certsUrl, + tokenURL: tokenURL, + certsURL: certsURL, } } @@ -30,14 +30,14 @@ func (a Application) ToSecret(name, ns string) kcorev1.Secret { Namespace: ns, }, Data: map[string][]byte{ - "client_id": []byte(a.clientId), + "client_id": []byte(a.clientID), "client_secret": []byte(a.clientSecret), - "token_url": []byte(a.tokenUrl), - "certs_url": []byte(a.certsUrl), + "token_url": []byte(a.tokenURL), + "certs_url": []byte(a.certsURL), }, } } -func (a Application) GetId() string { +func (a Application) GetID() string { return a.id } diff --git a/internal/skr/client.go b/internal/skr/client.go index 6533fe4..549e49d 100644 --- a/internal/skr/client.go +++ b/internal/skr/client.go @@ -5,6 +5,7 @@ import ( "fmt" eamias "github.com/kyma-project/eventing-auth-manager/internal/ias" + "github.com/pkg/errors" kcorev1 "k8s.io/api/core/v1" kapierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" @@ -28,8 +29,8 @@ type client struct { k8sClient kpkgclient.Client } -var NewClient = func(k8sClient kpkgclient.Client, skrClusterId string) (Client, error) { - kubeconfigSecretName := fmt.Sprintf("kubeconfig-%s", skrClusterId) +var NewClient = func(k8sClient kpkgclient.Client, skrClusterID string) (Client, error) { //nolint:gochecknoglobals // For mocking purposes. + kubeconfigSecretName := fmt.Sprintf("kubeconfig-%s", skrClusterID) secret := &kcorev1.Secret{} if err := k8sClient.Get(context.Background(), types.NamespacedName{Name: kubeconfigSecretName, Namespace: KcpNamespace}, secret); err != nil { @@ -38,7 +39,7 @@ var NewClient = func(k8sClient kpkgclient.Client, skrClusterId string) (Client, kubeconfig := secret.Data["config"] if len(kubeconfig) == 0 { - return nil, fmt.Errorf("failed to find SKR cluster kubeconfig in secret %s", kubeconfigSecretName) + return nil, errors.Errorf("failed to find SKR cluster kubeconfig in secret %s", kubeconfigSecretName) } config, err := clientcmd.RESTConfigFromKubeConfig(kubeconfig) diff --git a/internal/skr/client_test.go b/internal/skr/client_test.go index 448eb34..49b5830 100644 --- a/internal/skr/client_test.go +++ b/internal/skr/client_test.go @@ -13,10 +13,12 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" ) +var errGetSecret = errors.New("error on getting secret") + func Test_NewClient(t *testing.T) { type args struct { k8sClient kpkgclient.Client - skrClusterId string + skrClusterID string } tests := []struct { name string @@ -27,23 +29,23 @@ func Test_NewClient(t *testing.T) { name: "should return error when secret with kubeconfig is not found", args: args{ k8sClient: fake.NewClientBuilder().Build(), - skrClusterId: "test", + skrClusterID: "test", }, - wantError: errors.New("secrets \"kubeconfig-test\" not found"), + wantError: errors.New("secrets \"kubeconfig-test\" not found"), //nolint:goerr113 // used one time only in tests. }, { name: "should return error when secret doesn't contain config key", args: args{ k8sClient: fake.NewClientBuilder().WithObjects(&kcorev1.Secret{ObjectMeta: kmetav1.ObjectMeta{Name: "kubeconfig-test", Namespace: KcpNamespace}}).Build(), - skrClusterId: "test", + skrClusterID: "test", }, - wantError: errors.New("failed to find SKR cluster kubeconfig in secret kubeconfig-test"), + wantError: errors.New("failed to find SKR cluster kubeconfig in secret kubeconfig-test"), //nolint:goerr113 // used one time only in tests. }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { // when - _, err := NewClient(tt.args.k8sClient, tt.args.skrClusterId) + _, err := NewClient(tt.args.k8sClient, tt.args.skrClusterID) // then require.Error(t, err) @@ -84,10 +86,10 @@ func Test_client_DeleteSecret(t *testing.T) { fields: fields{ k8sClient: errorFakeClient{ Client: fake.NewClientBuilder().Build(), - errorOnGet: errors.New("error on getting secret"), + errorOnGet: errGetSecret, }, }, - wantErr: errors.New("error on getting secret"), + wantErr: errGetSecret, }, { name: "should ignore NotFound error when fetching secret", @@ -152,11 +154,11 @@ func Test_client_HasApplicationSecret(t *testing.T) { fields: fields{ k8sClient: errorFakeClient{ Client: fake.NewClientBuilder().Build(), - errorOnGet: errors.New("error on getting secret"), + errorOnGet: errGetSecret, }, }, want: false, - wantErr: errors.New("error on getting secret"), + wantErr: errGetSecret, }, { name: "should ignore NotFound error when fetching secret",