diff --git a/.gitignore b/.gitignore index a7c0480..68f56ee 100644 --- a/.gitignore +++ b/.gitignore @@ -26,3 +26,5 @@ Dockerfile.cross *~ vendor +.golangci.yaml +lint-report.json diff --git a/Makefile b/Makefile index 8947625..a8cdffe 100644 --- a/Makefile +++ b/Makefile @@ -161,3 +161,36 @@ $(CONTROLLER_GEN): $(LOCALBIN) envtest: $(ENVTEST) ## Download envtest-setup locally if necessary. $(ENVTEST): $(LOCALBIN) test -s $(LOCALBIN)/setup-envtest || GOBIN=$(LOCALBIN) go install sigs.k8s.io/controller-runtime/tools/setup-envtest@latest + +# Lint issue category +CATEGORY = "default" + +.PHONY: lint +lint: ## Check lint issues using `golangci-lint` + golangci-lint run --timeout 5m --config=./.golangci.yaml + +.PHONY: lint-compact +lint-compact: ## Check lint issues using `golangci-lint` in compact result format + golangci-lint run --timeout 5m --config=./.golangci.yaml --print-issued-lines=false + +.PHONY: lint-fix +lint-fix: ## Check and fix lint issues using `golangci-lint` + golangci-lint run --fix --timeout 5m --config=./.golangci.yaml + +.PHONY: lint-report +lint-report: ## Check lint issues using `golangci-lint` then export them to a file, then print the list of linters used + golangci-lint run --timeout 5m --config=./.golangci.yaml --issues-exit-code 0 --out-format json > ./lint-report.json + +.PHONY: lint-report-issue-category +lint-report-issue-category: ## Get lint issues categories + make lint-report-clean + make lint-report + cat ./lint-report.json | jq '.Issues[].FromLinter' | jq -s 'map({(.):1})|add|keys_unsorted' + +.PHONY: lint-report-get-category +lint-report-get-category: ## Get lint issues by category + cat ./lint-report.json | jq --arg CATEGORY $$CATEGORY '.Issues[] | select(.FromLinter==$$CATEGORY)' + +.PHONY: lint-report-clean +lint-report-clean: ## Clean lint report + rm -f ./lint-report.json diff --git a/api/v1alpha1/eventingauth_types.go b/api/v1alpha1/eventingauth_types.go index 729cbf4..9e6835b 100644 --- a/api/v1alpha1/eventingauth_types.go +++ b/api/v1alpha1/eventingauth_types.go @@ -28,13 +28,13 @@ const ( StateNotReady State = "NotReady" ) -// EventingAuthSpec defines the desired state of EventingAuth +// EventingAuthSpec defines the desired state of EventingAuth. type EventingAuthSpec struct { // INSERT ADDITIONAL SPEC FIELDS - desired state of cluster // Important: Run "make" to regenerate code after modifying this file } -// EventingAuthStatus defines the observed state of EventingAuth +// EventingAuthStatus defines the observed state of EventingAuth. type EventingAuthStatus struct { // State signifies current state of CustomObject. Value // can be one of ("Ready", "NotReady"). @@ -69,7 +69,7 @@ type AuthSecret struct { //+kubebuilder:subresource:status //+kubebuilder:printcolumn:name="State",type="string",JSONPath=".status.state" -// EventingAuth is the Schema for the eventingauths API +// EventingAuth is the Schema for the eventingauths API. type EventingAuth struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` @@ -80,7 +80,7 @@ type EventingAuth struct { //+kubebuilder:object:root=true -// EventingAuthList contains a list of EventingAuth +// EventingAuthList contains a list of EventingAuth. type EventingAuthList struct { metav1.TypeMeta `json:",inline"` metav1.ListMeta `json:"metadata,omitempty"` diff --git a/api/v1alpha1/groupversion_info.go b/api/v1alpha1/groupversion_info.go index d5f916d..34c9e3d 100644 --- a/api/v1alpha1/groupversion_info.go +++ b/api/v1alpha1/groupversion_info.go @@ -25,10 +25,10 @@ import ( ) var ( - // GroupVersion is group version used to register these objects + // GroupVersion is group version used to register these objects. GroupVersion = schema.GroupVersion{Group: "operator.kyma-project.io", Version: "v1alpha1"} - // SchemeBuilder is used to add go types to the GroupVersionKind scheme + // SchemeBuilder is used to add go types to the GroupVersionKind scheme. SchemeBuilder = &scheme.Builder{GroupVersion: GroupVersion} // AddToScheme adds the types in this group-version to the given scheme. diff --git a/api/v1alpha1/status_test.go b/api/v1alpha1/status_test.go index aadc85b..11125cb 100644 --- a/api/v1alpha1/status_test.go +++ b/api/v1alpha1/status_test.go @@ -41,7 +41,7 @@ func Test_DetermineEventingAuthState(t *testing.T) { name: "Should not be ready if only application condition is available", givenStatus: EventingAuthStatus{ Conditions: []metav1.Condition{ - metav1.Condition{ + { Type: string(ConditionApplicationReady), Status: metav1.ConditionTrue, }, @@ -53,7 +53,7 @@ func Test_DetermineEventingAuthState(t *testing.T) { name: "Should not be ready if only secret condition is available", givenStatus: EventingAuthStatus{ Conditions: []metav1.Condition{ - metav1.Condition{ + { Type: string(ConditionSecretReady), Status: metav1.ConditionTrue, }, @@ -81,26 +81,26 @@ func Test_IsEventingAuthStatusEqual(t *testing.T) { }{ { name: "Should status be equal", - givenOldStatus: createEventingAuthStatus(metav1.ConditionTrue, metav1.ConditionTrue, "mock-app1", "mock-secret-ns1", StateReady), - givenNewStatus: createEventingAuthStatus(metav1.ConditionTrue, metav1.ConditionTrue, "mock-app1", "mock-secret-ns1", StateReady), + givenOldStatus: createEventingAuthStatus(metav1.ConditionTrue, "mock-app1", "mock-secret-ns1", StateReady), + givenNewStatus: createEventingAuthStatus(metav1.ConditionTrue, "mock-app1", "mock-secret-ns1", StateReady), result: true, }, { name: "Should not be equal as conditions are different", - givenOldStatus: createEventingAuthStatus(metav1.ConditionTrue, metav1.ConditionFalse, "mock-app1", "mock-secret-ns1", StateReady), - givenNewStatus: createEventingAuthStatus(metav1.ConditionTrue, metav1.ConditionTrue, "mock-app1", "mock-secret-ns1", StateReady), + givenOldStatus: createEventingAuthStatus(metav1.ConditionFalse, "mock-app1", "mock-secret-ns1", StateReady), + givenNewStatus: createEventingAuthStatus(metav1.ConditionTrue, "mock-app1", "mock-secret-ns1", StateReady), result: false, }, { name: "Should not be equal as ias app and secret names are different", - givenOldStatus: createEventingAuthStatus(metav1.ConditionTrue, metav1.ConditionTrue, "mock-app1", "mock-secret-ns1", StateReady), - givenNewStatus: createEventingAuthStatus(metav1.ConditionTrue, metav1.ConditionTrue, "mock-app2", "mock-secret-ns2", StateReady), + givenOldStatus: createEventingAuthStatus(metav1.ConditionTrue, "mock-app1", "mock-secret-ns1", StateReady), + givenNewStatus: createEventingAuthStatus(metav1.ConditionTrue, "mock-app2", "mock-secret-ns2", StateReady), result: false, }, { name: "Should not be equal as state is different", - givenOldStatus: createEventingAuthStatus(metav1.ConditionTrue, metav1.ConditionTrue, "mock-app1", "mock-secret-ns1", StateReady), - givenNewStatus: createEventingAuthStatus(metav1.ConditionTrue, metav1.ConditionTrue, "mock-app1", "mock-secret-ns1", StateNotReady), + givenOldStatus: createEventingAuthStatus(metav1.ConditionTrue, "mock-app1", "mock-secret-ns1", StateReady), + givenNewStatus: createEventingAuthStatus(metav1.ConditionTrue, "mock-app1", "mock-secret-ns1", StateNotReady), result: false, }, } @@ -125,7 +125,7 @@ func Test_MakeApplicationReadyCondition(t *testing.T) { name: "Should application ready condition be added", givenEventingAuth: createEventingAuthWith(EventingAuthStatus{Conditions: []metav1.Condition{}}), wantConditions: []metav1.Condition{ - metav1.Condition{ + { Type: string(ConditionApplicationReady), Status: metav1.ConditionTrue, Reason: ConditionReasonApplicationCreated, @@ -136,7 +136,7 @@ func Test_MakeApplicationReadyCondition(t *testing.T) { { name: "Should update false condition to true if no error", givenEventingAuth: createEventingAuthWith(EventingAuthStatus{Conditions: []metav1.Condition{ - metav1.Condition{ + { Type: string(ConditionApplicationReady), Status: metav1.ConditionFalse, Reason: ConditionReasonApplicationCreationFailed, @@ -144,7 +144,7 @@ func Test_MakeApplicationReadyCondition(t *testing.T) { }, }}), wantConditions: []metav1.Condition{ - metav1.Condition{ + { Type: string(ConditionApplicationReady), Status: metav1.ConditionTrue, Reason: ConditionReasonApplicationCreated, @@ -155,7 +155,7 @@ func Test_MakeApplicationReadyCondition(t *testing.T) { { name: "Should update condition to false if error occurs", givenEventingAuth: createEventingAuthWith(EventingAuthStatus{Conditions: []metav1.Condition{ - metav1.Condition{ + { Type: string(ConditionApplicationReady), Status: metav1.ConditionTrue, Reason: ConditionReasonApplicationCreated, @@ -164,7 +164,7 @@ func Test_MakeApplicationReadyCondition(t *testing.T) { }}), givenErr: fmt.Errorf(mockErrorMessage), wantConditions: []metav1.Condition{ - metav1.Condition{ + { Type: string(ConditionApplicationReady), Status: metav1.ConditionFalse, Reason: ConditionReasonApplicationCreationFailed, @@ -193,7 +193,7 @@ func Test_MakeSecretReadyCondition(t *testing.T) { { name: "Should application ready condition be added", givenEventingAuth: createEventingAuthWith(EventingAuthStatus{Conditions: []metav1.Condition{ - metav1.Condition{ + { Type: string(ConditionApplicationReady), Status: metav1.ConditionTrue, Reason: ConditionReasonApplicationCreated, @@ -201,13 +201,13 @@ func Test_MakeSecretReadyCondition(t *testing.T) { }, }}), wantConditions: []metav1.Condition{ - metav1.Condition{ + { Type: string(ConditionApplicationReady), Status: metav1.ConditionTrue, Reason: ConditionReasonApplicationCreated, Message: ConditionMessageApplicationCreated, }, - metav1.Condition{ + { Type: string(ConditionSecretReady), Status: metav1.ConditionTrue, Reason: ConditionReasonSecretCreated, @@ -218,13 +218,13 @@ func Test_MakeSecretReadyCondition(t *testing.T) { { name: "Should update not ready secret condition to ready condition", givenEventingAuth: createEventingAuthWith(EventingAuthStatus{Conditions: []metav1.Condition{ - metav1.Condition{ + { Type: string(ConditionApplicationReady), Status: metav1.ConditionTrue, Reason: ConditionReasonApplicationCreated, Message: ConditionMessageApplicationCreated, }, - metav1.Condition{ + { Type: string(ConditionSecretReady), Status: metav1.ConditionFalse, Reason: ConditionReasonSecretCreationFailed, @@ -232,13 +232,13 @@ func Test_MakeSecretReadyCondition(t *testing.T) { }, }}), wantConditions: []metav1.Condition{ - metav1.Condition{ + { Type: string(ConditionApplicationReady), Status: metav1.ConditionTrue, Reason: ConditionReasonApplicationCreated, Message: ConditionMessageApplicationCreated, }, - metav1.Condition{ + { Type: string(ConditionSecretReady), Status: metav1.ConditionTrue, Reason: ConditionReasonSecretCreated, @@ -249,13 +249,13 @@ func Test_MakeSecretReadyCondition(t *testing.T) { { name: "Should update ready secret condition to not ready when error occurs", givenEventingAuth: createEventingAuthWith(EventingAuthStatus{Conditions: []metav1.Condition{ - metav1.Condition{ + { Type: string(ConditionApplicationReady), Status: metav1.ConditionTrue, Reason: ConditionReasonApplicationCreated, Message: ConditionMessageApplicationCreated, }, - metav1.Condition{ + { Type: string(ConditionSecretReady), Status: metav1.ConditionTrue, Reason: ConditionReasonSecretCreated, @@ -264,13 +264,13 @@ func Test_MakeSecretReadyCondition(t *testing.T) { }}), givenErr: fmt.Errorf(mockErrorMessage), wantConditions: []metav1.Condition{ - metav1.Condition{ + { Type: string(ConditionApplicationReady), Status: metav1.ConditionTrue, Reason: ConditionReasonApplicationCreated, Message: ConditionMessageApplicationCreated, }, - metav1.Condition{ + { Type: string(ConditionSecretReady), Status: metav1.ConditionFalse, Reason: ConditionReasonSecretCreationFailed, @@ -303,13 +303,13 @@ func Test_UpdateConditionAndState(t *testing.T) { name: "Should update state to NotReady as error occurs", givenEventingAuth: createEventingAuthWith(EventingAuthStatus{ Conditions: []metav1.Condition{ - metav1.Condition{ + { Type: string(ConditionApplicationReady), Status: metav1.ConditionTrue, Reason: ConditionReasonApplicationCreated, Message: ConditionMessageApplicationCreated, }, - metav1.Condition{ + { Type: string(ConditionSecretReady), Status: metav1.ConditionTrue, Reason: ConditionReasonSecretCreated, @@ -322,13 +322,13 @@ func Test_UpdateConditionAndState(t *testing.T) { givenErr: fmt.Errorf(mockErrorMessage), wantStatus: EventingAuthStatus{ Conditions: []metav1.Condition{ - metav1.Condition{ + { Type: string(ConditionApplicationReady), Status: metav1.ConditionTrue, Reason: ConditionReasonApplicationCreated, Message: ConditionMessageApplicationCreated, }, - metav1.Condition{ + { Type: string(ConditionSecretReady), Status: metav1.ConditionFalse, Reason: ConditionReasonSecretCreationFailed, @@ -342,13 +342,13 @@ func Test_UpdateConditionAndState(t *testing.T) { name: "Should update state to Ready as no error occurs", givenEventingAuth: createEventingAuthWith(EventingAuthStatus{ Conditions: []metav1.Condition{ - metav1.Condition{ + { Type: string(ConditionApplicationReady), Status: metav1.ConditionTrue, Reason: ConditionReasonApplicationCreated, Message: ConditionMessageApplicationCreated, }, - metav1.Condition{ + { Type: string(ConditionSecretReady), Status: metav1.ConditionFalse, Reason: ConditionReasonSecretCreationFailed, @@ -360,13 +360,13 @@ func Test_UpdateConditionAndState(t *testing.T) { conditionType: ConditionSecretReady, wantStatus: EventingAuthStatus{ Conditions: []metav1.Condition{ - metav1.Condition{ + { Type: string(ConditionApplicationReady), Status: metav1.ConditionTrue, Reason: ConditionReasonApplicationCreated, Message: ConditionMessageApplicationCreated, }, - metav1.Condition{ + { Type: string(ConditionSecretReady), Status: metav1.ConditionTrue, Reason: ConditionReasonSecretCreated, @@ -380,7 +380,7 @@ func Test_UpdateConditionAndState(t *testing.T) { name: "Should update state to NotReady due to missing secret condition", givenEventingAuth: createEventingAuthWith(EventingAuthStatus{ Conditions: []metav1.Condition{ - metav1.Condition{ + { Type: string(ConditionApplicationReady), Status: metav1.ConditionFalse, Reason: ConditionReasonApplicationCreated, @@ -392,7 +392,7 @@ func Test_UpdateConditionAndState(t *testing.T) { conditionType: ConditionApplicationReady, wantStatus: EventingAuthStatus{ Conditions: []metav1.Condition{ - metav1.Condition{ + { Type: string(ConditionApplicationReady), Status: metav1.ConditionTrue, Reason: ConditionReasonApplicationCreated, @@ -406,7 +406,7 @@ func Test_UpdateConditionAndState(t *testing.T) { name: "Should fail if invalid condition type is provided", givenEventingAuth: createEventingAuthWith(EventingAuthStatus{ Conditions: []metav1.Condition{ - metav1.Condition{ + { Type: string(ConditionApplicationReady), Status: metav1.ConditionFalse, Reason: ConditionReasonApplicationCreated, @@ -438,11 +438,11 @@ func Test_UpdateConditionAndState(t *testing.T) { func createTwoTrueConditions() []metav1.Condition { return []metav1.Condition{ - metav1.Condition{ + { Type: string(ConditionApplicationReady), Status: metav1.ConditionTrue, }, - metav1.Condition{ + { Type: string(ConditionSecretReady), Status: metav1.ConditionTrue, }, @@ -450,11 +450,11 @@ func createTwoTrueConditions() []metav1.Condition { } func createTwoFalseConditions() []metav1.Condition { return []metav1.Condition{ - metav1.Condition{ + { Type: string(ConditionApplicationReady), Status: metav1.ConditionFalse, }, - metav1.Condition{ + { Type: string(ConditionSecretReady), Status: metav1.ConditionFalse, }, @@ -463,25 +463,25 @@ func createTwoFalseConditions() []metav1.Condition { func createTwoConditionsWithOneFalse() []metav1.Condition { return []metav1.Condition{ - metav1.Condition{ + { Type: string(ConditionApplicationReady), Status: metav1.ConditionTrue, }, - metav1.Condition{ + { Type: string(ConditionSecretReady), Status: metav1.ConditionFalse, }, } } -func createEventingAuthStatus(appReadyStatus, secretReadyStatus metav1.ConditionStatus, appName, secretNSName string, state State) EventingAuthStatus { +func createEventingAuthStatus(secretReadyStatus metav1.ConditionStatus, appName, secretNSName string, state State) EventingAuthStatus { return EventingAuthStatus{ Conditions: []metav1.Condition{ - metav1.Condition{ + { Type: string(ConditionApplicationReady), - Status: appReadyStatus, + Status: metav1.ConditionTrue, }, - metav1.Condition{ + { Type: string(ConditionSecretReady), Status: secretReadyStatus, }, diff --git a/cmd/main.go b/cmd/main.go index a3a9e0a..1029f10 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -20,13 +20,9 @@ import ( "flag" "os" + operatorv1alpha1 "github.com/kyma-project/eventing-auth-manager/api/v1alpha1" "github.com/kyma-project/eventing-auth-manager/controllers" kymav1beta1 "github.com/kyma-project/lifecycle-manager/api/v1beta1" - - // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) - // to ensure that exec-entrypoint and run can make use of them. - _ "k8s.io/client-go/plugin/pkg/client/auth" - "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" @@ -36,8 +32,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/metrics/server" "sigs.k8s.io/controller-runtime/pkg/webhook" - operatorv1alpha1 "github.com/kyma-project/eventing-auth-manager/api/v1alpha1" - //+kubebuilder:scaffold:imports + // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) + // to ensure that exec-entrypoint and run can make use of them. + _ "k8s.io/client-go/plugin/pkg/client/auth" ) const webhookPort = 9443 diff --git a/controllers/eventingauth_controller.go b/controllers/eventingauth_controller.go index 77c5f71..73ac1a3 100644 --- a/controllers/eventingauth_controller.go +++ b/controllers/eventingauth_controller.go @@ -38,10 +38,10 @@ const ( iasCredsSecretNamespace string = "IAS_CREDS_SECRET_NAMESPACE" iasCredsSecretName string = "IAS_CREDS_SECRET_NAME" defaultIasCredsNamespaceName string = "kcp-system" - DefaultIasCredsSecretName string = "eventing-auth-ias-creds" + DefaultIasCredsSecretName string = "eventing-auth-ias-creds" //nolint:gosec ) -// eventingAuthReconciler reconciles a EventingAuth object +// eventingAuthReconciler reconciles a EventingAuth object. type eventingAuthReconciler struct { client.Client Scheme *runtime.Scheme @@ -168,7 +168,7 @@ func (r *eventingAuthReconciler) getIasClient() (ias.Client, error) { // update IAS client if credentials are changed iasClient, err := ias.NewClient(newIasCredentials.URL, newIasCredentials.Username, newIasCredentials.Password) if err != nil { - return nil, fmt.Errorf("failed to createa a new IAS client: %v", err) + return nil, fmt.Errorf("failed to createa a new IAS client: %w", err) } return iasClient, nil } @@ -185,13 +185,13 @@ func getIasSecretNamespaceAndNameConfigs() (string, string) { return namespace, name } -// Adds the finalizer if none exists +// Adds the finalizer if none exists. func (r *eventingAuthReconciler) addFinalizer(ctx context.Context, cr *operatorv1alpha1.EventingAuth) error { if !controllerutil.ContainsFinalizer(cr, eventingAuthFinalizerName) { ctrl.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: %v", err) + return fmt.Errorf("failed to add finalizer: %w", err) } } return nil @@ -203,7 +203,7 @@ func (r *eventingAuthReconciler) handleDeletion(ctx context.Context, iasClient i 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: %v", err) + return fmt.Errorf("failed to delete IAS Application: %w", err) } ctrl.Log.Info("Deleted IAS application", "eventingAuth", cr.Name, "namespace", cr.Namespace) @@ -218,7 +218,7 @@ func (r *eventingAuthReconciler) handleDeletion(ctx context.Context, iasClient i // 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: %v", err) + return fmt.Errorf("failed to remove finalizer: %w", err) } } return nil @@ -263,7 +263,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: %v", err) + return fmt.Errorf("failed to update EventingAuth status: %w", err) } return nil diff --git a/controllers/eventingauth_controller_test.go b/controllers/eventingauth_controller_test.go index 25d5625..586e2c4 100644 --- a/controllers/eventingauth_controller_test.go +++ b/controllers/eventingauth_controller_test.go @@ -8,19 +8,20 @@ import ( "github.com/google/uuid" "github.com/kyma-project/eventing-auth-manager/api/v1alpha1" "github.com/kyma-project/eventing-auth-manager/internal/skr" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - . "github.com/onsi/gomega/gstruct" gtypes "github.com/onsi/gomega/types" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrlClient "sigs.k8s.io/controller-runtime/pkg/client" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "github.com/onsi/gomega/gstruct" ) var appSecretObjectKey = ctrlClient.ObjectKey{Name: skr.ApplicationSecretName, Namespace: skr.ApplicationSecretNamespace} -// Since all tests use the same target cluster and therefore share the same application secret, they need to be executed serially +// Since all tests use the same target cluster and therefore share the same application secret, they need to be executed serially. var _ = Describe("EventingAuth Controller happy tests", Serial, Ordered, func() { BeforeAll(func() { // We allow the happy path tests to use the real IAS client if the test env vars are set @@ -261,7 +262,7 @@ func deleteEventingAuthAndVerify(e *v1alpha1.EventingAuth) { Eventually(func(g Gomega) { latestEAuth := &v1alpha1.EventingAuth{} err := k8sClient.Get(context.TODO(), ctrlClient.ObjectKeyFromObject(e), latestEAuth) - g.Expect(err).ShouldNot(BeNil()) + g.Expect(err).Should(HaveOccurred()) g.Expect(errors.IsNotFound(err)).Should(BeTrue()) }, defaultTimeout).Should(Succeed()) } @@ -279,7 +280,7 @@ func deleteApplicationSecretOnTargetCluster() { } } -func createKubeconfigSecret(crName string) *corev1.Secret { +func createKubeconfigSecret(crName string) { By("Creating secret with kubeconfig of target cluster") kubeconfigSecret := corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -291,10 +292,9 @@ func createKubeconfigSecret(crName string) *corev1.Secret { }, } Expect(k8sClient.Create(context.TODO(), &kubeconfigSecret)).Should(Succeed()) - return &kubeconfigSecret } -func deleteKubeconfigSecret(crName string) *corev1.Secret { +func deleteKubeconfigSecret(crName string) { By("Deleting secret with kubeconfig of target cluster") kubeconfigSecret := corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -308,5 +308,4 @@ func deleteKubeconfigSecret(crName string) *corev1.Secret { g.Expect(errors.IsNotFound(err)).Should(BeTrue()) } }, defaultTimeout).Should(Succeed()) - return &kubeconfigSecret } diff --git a/controllers/kyma_controller.go b/controllers/kyma_controller.go index a5705fe..edc8aa5 100644 --- a/controllers/kyma_controller.go +++ b/controllers/kyma_controller.go @@ -17,7 +17,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log" ) -// KymaReconciler reconciles a Kyma resource +// KymaReconciler reconciles a Kyma resource. type KymaReconciler struct { client.Client Scheme *runtime.Scheme @@ -67,11 +67,11 @@ func (r *KymaReconciler) createEventingAuth(ctx context.Context, kyma *kymav1bet } err = r.Client.Create(ctx, eventingAuth) if err != nil { - return fmt.Errorf("failed to create EventingAuth resource: %v", err) + return fmt.Errorf("failed to create EventingAuth resource: %w", err) } return nil } - return fmt.Errorf("failed to retrieve EventingAuth resource: %v", err) + return fmt.Errorf("failed to retrieve EventingAuth resource: %w", err) } return nil } diff --git a/controllers/kyma_controller_test.go b/controllers/kyma_controller_test.go index 3da8745..a77c6b0 100644 --- a/controllers/kyma_controller_test.go +++ b/controllers/kyma_controller_test.go @@ -10,16 +10,17 @@ import ( "github.com/kyma-project/eventing-auth-manager/internal/skr" kymav1beta1 "github.com/kyma-project/lifecycle-manager/api/v1beta1" kymav1beta2 "github.com/kyma-project/lifecycle-manager/api/v1beta2" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ctrlClient "sigs.k8s.io/controller-runtime/pkg/client" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" ) -// Since all tests use the same target cluster and therefore share the same application secret, they need to be executed serially +// Since all tests use the same target cluster and therefore share the same application secret, they need to be executed serially. var _ = Describe("Kyma Controller", Serial, Ordered, func() { var kyma *kymav1beta1.Kyma var crName string @@ -102,7 +103,7 @@ func deleteKymaResource(kyma *kymav1beta1.Kyma) { eventingAuth := &v1alpha1.EventingAuth{} err = k8sClient.Get(context.TODO(), types.NamespacedName{Namespace: skr.KcpNamespace, Name: kyma.Name}, eventingAuth) if useExistingCluster { - g.Expect(err).ShouldNot(BeNil()) + g.Expect(err).Should(HaveOccurred()) g.Expect(errors.IsNotFound(err)).To(BeTrue()) } else { // clean up EventingAuth for not real cluster diff --git a/controllers/stubs_test.go b/controllers/stubs_test.go index a7a0476..46922d2 100644 --- a/controllers/stubs_test.go +++ b/controllers/stubs_test.go @@ -8,9 +8,10 @@ import ( "github.com/google/uuid" "github.com/kyma-project/eventing-auth-manager/internal/ias" "github.com/kyma-project/eventing-auth-manager/internal/skr" - . "github.com/onsi/ginkgo/v2" v1 "k8s.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" + + . "github.com/onsi/ginkgo/v2" ) var ( diff --git a/controllers/suite_test.go b/controllers/suite_test.go index b7916cd..fb61370 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -25,22 +25,18 @@ import ( "testing" "time" + operatorv1alpha1 "github.com/kyma-project/eventing-auth-manager/api/v1alpha1" + "github.com/kyma-project/eventing-auth-manager/controllers" + "github.com/kyma-project/eventing-auth-manager/internal/skr" kymav1beta1 "github.com/kyma-project/lifecycle-manager/api/v1beta1" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" - - "github.com/kyma-project/eventing-auth-manager/controllers" - "github.com/kyma-project/eventing-auth-manager/internal/skr" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - corev1 "k8s.io/api/core/v1" - - "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" @@ -48,7 +44,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log/zap" "sigs.k8s.io/controller-runtime/pkg/metrics/server" - operatorv1alpha1 "github.com/kyma-project/eventing-auth-manager/api/v1alpha1" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" ) // These tests use Ginkgo (BDD-style Go testing framework). Refer to diff --git a/internal/ias/client.go b/internal/ias/client.go index 2882807..6ec3340 100644 --- a/internal/ias/client.go +++ b/internal/ias/client.go @@ -34,8 +34,9 @@ var NewClient = func(iasTenantUrl, user, password string) (Client, error) { return nil, err } + const timeout = time.Second * 5 oidcHttpClient := &http.Client{ - Timeout: time.Second * 5, + Timeout: timeout, } return &client{ @@ -67,7 +68,6 @@ func (c *client) GetCredentials() *Credentials { // CreateApplication creates an application in IAS. This function is not idempotent, because if an application with the specified // name already exists, it will be deleted and recreated. func (c *client) CreateApplication(ctx context.Context, name string) (Application, error) { - existingApp, err := c.getApplicationByName(ctx, name) if err != nil { return Application{}, err @@ -171,7 +171,7 @@ func (c *client) getApplicationByName(ctx context.Context, name string) (*api.Ap // This is not documented in the API, but the actual API returned 404 if no applications were found. if res.StatusCode() == http.StatusNotFound { - return nil, nil + return nil, nil //nolint:nilnil } if res.StatusCode() != http.StatusOK { @@ -184,14 +184,14 @@ func (c *client) getApplicationByName(ctx context.Context, name string) (*api.Ap // Since the handling of the 404 status is not documented, we also handle the case where no more applications are found, // because we do not know what the expected behavior should be. case 0: - return nil, nil + return nil, nil //nolint:nilnil case 1: return &(*res.JSON200.Applications)[0], nil default: return nil, fmt.Errorf("found multiple applications with the same name %s", name) } } - return nil, nil + return nil, nil //nolint:nilnil } func (c *client) createNewApplication(ctx context.Context, name string) (uuid.UUID, error) { diff --git a/internal/ias/client_test.go b/internal/ias/client_test.go index d4cf87a..89b22e9 100644 --- a/internal/ias/client_test.go +++ b/internal/ias/client_test.go @@ -35,8 +35,8 @@ func Test_CreateApplication(t *testing.T) { mockGetAllApplicationsWithResponseStatusOkEmptyResponse(&clientMock) mockCreateApplicationWithResponseStatusCreated(&clientMock, appId.String()) - mockCreateApiSecretWithResponseStatusCreated(&clientMock, appId, "clientSecretMock") - mockGetApplicationWithResponseStatusOK(&clientMock, appId, "clientIdMock") + mockCreateApiSecretWithResponseStatusCreated(&clientMock, appId) + mockGetApplicationWithResponseStatusOK(&clientMock, appId) return &clientMock }, @@ -61,8 +61,8 @@ func Test_CreateApplication(t *testing.T) { mockGetAllApplicationsWithResponseStatusNotFound(&clientMock) mockCreateApplicationWithResponseStatusCreated(&clientMock, appId.String()) - mockCreateApiSecretWithResponseStatusCreated(&clientMock, appId, "clientSecretMock") - mockGetApplicationWithResponseStatusOK(&clientMock, appId, "clientIdMock") + mockCreateApiSecretWithResponseStatusCreated(&clientMock, appId) + mockGetApplicationWithResponseStatusOK(&clientMock, appId) return &clientMock }, @@ -90,8 +90,8 @@ func Test_CreateApplication(t *testing.T) { newAppId := uuid.MustParse("90764f89-f041-4ccf-8da9-7a7c2d60d7fc") mockCreateApplicationWithResponseStatusCreated(&clientMock, newAppId.String()) - mockCreateApiSecretWithResponseStatusCreated(&clientMock, newAppId, "clientSecretMock") - mockGetApplicationWithResponseStatusOK(&clientMock, newAppId, "clientIdMock") + mockCreateApiSecretWithResponseStatusCreated(&clientMock, newAppId) + mockGetApplicationWithResponseStatusOK(&clientMock, newAppId) return &clientMock }, @@ -124,7 +124,6 @@ func Test_CreateApplication(t *testing.T) { { name: "should return error when application ID can't be retrieved from location header", givenApiMock: func() *mocks.ClientWithResponsesInterface { - clientMock := mocks.ClientWithResponsesInterface{} mockGetAllApplicationsWithResponseStatusOkEmptyResponse(&clientMock) mockCreateApplicationWithResponseStatusCreated(&clientMock, "non-uuid-application-id") @@ -196,7 +195,7 @@ func Test_CreateApplication(t *testing.T) { mockGetAllApplicationsWithResponseStatusOkEmptyResponse(&clientMock) mockCreateApplicationWithResponseStatusCreated(&clientMock, appId.String()) - mockCreateApiSecretWithResponseStatusCreated(&clientMock, appId, "clientSecretMock") + mockCreateApiSecretWithResponseStatusCreated(&clientMock, appId) mockGetApplicationWithResponseStatusInternalServerError(&clientMock) return &clientMock @@ -211,8 +210,8 @@ func Test_CreateApplication(t *testing.T) { mockGetAllApplicationsWithResponseStatusOkEmptyResponse(&clientMock) mockCreateApplicationWithResponseStatusCreated(&clientMock, appId.String()) - mockCreateApiSecretWithResponseStatusCreated(&clientMock, appId, "clientSecretMock") - mockGetApplicationWithResponseStatusOK(&clientMock, appId, "clientIdMock") + mockCreateApiSecretWithResponseStatusCreated(&clientMock, appId) + mockGetApplicationWithResponseStatusOK(&clientMock, appId) return &clientMock }, @@ -227,8 +226,8 @@ func Test_CreateApplication(t *testing.T) { mockGetAllApplicationsWithResponseStatusOkEmptyResponse(&clientMock) mockCreateApplicationWithResponseStatusCreated(&clientMock, appId.String()) - mockCreateApiSecretWithResponseStatusCreated(&clientMock, appId, "clientSecretMock") - mockGetApplicationWithResponseStatusOK(&clientMock, appId, "clientIdMock") + mockCreateApiSecretWithResponseStatusCreated(&clientMock, appId) + mockGetApplicationWithResponseStatusOK(&clientMock, appId) return &clientMock }, @@ -243,8 +242,8 @@ func Test_CreateApplication(t *testing.T) { mockGetAllApplicationsWithResponseStatusOkEmptyResponse(&clientMock) mockCreateApplicationWithResponseStatusCreated(&clientMock, appId.String()) - mockCreateApiSecretWithResponseStatusCreated(&clientMock, appId, "clientSecretMock") - mockGetApplicationWithResponseStatusOK(&clientMock, appId, "clientIdMock") + mockCreateApiSecretWithResponseStatusCreated(&clientMock, appId) + mockGetApplicationWithResponseStatusOK(&clientMock, appId) return &clientMock }, @@ -409,11 +408,11 @@ func mockGetAllApplicationsWithResponseStatusOkEmptyResponse(clientMock *mocks.C } func mockGetAllApplicationsWithResponseStatusOk(clientMock *mocks.ClientWithResponsesInterface, appIds ...uuid.UUID) { - appResponses := make([]api.ApplicationResponse, 0) for _, appId := range appIds { + id := appId appResponses = append(appResponses, api.ApplicationResponse{ - Id: &appId, + Id: &id, }) } @@ -459,8 +458,9 @@ func mockCreateApiSecretWithResponseStatusInternalServerError(clientMock *mocks. }, nil) } -func mockCreateApiSecretWithResponseStatusCreated(clientMock *mocks.ClientWithResponsesInterface, appId uuid.UUID, clientSecretMock string) { +func mockCreateApiSecretWithResponseStatusCreated(clientMock *mocks.ClientWithResponsesInterface, appId uuid.UUID) { d := "eventing-auth-manager" + s := "clientSecretMock" secretRequest := api.CreateApiSecretJSONRequestBody{ AuthorizationScopes: &[]api.AuthorizationScope{"oAuth"}, Description: &d, @@ -471,7 +471,7 @@ func mockCreateApiSecretWithResponseStatusCreated(clientMock *mocks.ClientWithRe StatusCode: http.StatusCreated, }, JSON201: &api.ApiSecretResponse{ - Secret: &clientSecretMock, + Secret: &s, }, }, nil) } @@ -485,7 +485,8 @@ func mockGetApplicationWithResponseStatusInternalServerError(clientMock *mocks.C }, nil) } -func mockGetApplicationWithResponseStatusOK(clientMock *mocks.ClientWithResponsesInterface, appId uuid.UUID, clientIdMock string) { +func mockGetApplicationWithResponseStatusOK(clientMock *mocks.ClientWithResponsesInterface, appId uuid.UUID) { + cID := "clientIdMock" clientMock.On("GetApplicationWithResponse", mock.Anything, appId, mock.Anything). Return(&api.GetApplicationResponse{ HTTPResponse: &http.Response{ @@ -493,7 +494,7 @@ func mockGetApplicationWithResponseStatusOK(clientMock *mocks.ClientWithResponse }, JSON200: &api.ApplicationResponse{ UrnSapIdentityApplicationSchemasExtensionSci10Authentication: &api.AuthenticationSchema{ - ClientId: &clientIdMock, + ClientId: &cID, }, }, }, nil) @@ -527,6 +528,7 @@ func mockDeleteApplicationWithResponseStatusNotFound(clientMock *mocks.ClientWit } func mockClient(t *testing.T, tokenUrl, jwksURI *string) *oidcmocks.Client { + t.Helper() clientMock := oidcmocks.NewClient(t) clientMock.On("GetTokenEndpoint", mock.Anything).Return(tokenUrl, nil) clientMock.On("GetJWKSURI", mock.Anything).Return(jwksURI, nil) @@ -534,6 +536,7 @@ func mockClient(t *testing.T, tokenUrl, jwksURI *string) *oidcmocks.Client { } func mockGetTokenEndpoint(t *testing.T, tokenUrl *string) *oidcmocks.Client { + t.Helper() clientMock := oidcmocks.NewClient(t) 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 66dc6d1..86e60af 100644 --- a/internal/ias/ias_config.go +++ b/internal/ias/ias_config.go @@ -64,6 +64,6 @@ var ReadCredentials = func(namespace, name string, k8sClient ctlrClient.Client) if errors != nil { return nil, errors } - iasConfig := NewCredentials(string(url[:]), string(username[:]), string(password[:])) + iasConfig := NewCredentials(string(url), string(username), string(password)) return iasConfig, nil } diff --git a/internal/ias/internal/oidc/oidc-client_test.go b/internal/ias/internal/oidc/oidc-client_test.go index a946c2a..a053210 100644 --- a/internal/ias/internal/oidc/oidc-client_test.go +++ b/internal/ias/internal/oidc/oidc-client_test.go @@ -8,11 +8,10 @@ import ( "net/http" "testing" + "github.com/kyma-project/eventing-auth-manager/internal/ias/internal/oidc" "github.com/stretchr/testify/require" "k8s.io/client-go/rest/fake" "k8s.io/utils/ptr" - - "github.com/kyma-project/eventing-auth-manager/internal/ias/internal/oidc" ) const oidcConfigMock = `{"token_endpoint":"https://domain-url.com/token"}`