From 8d62ae04a0db42e9374ea7e9c1fe682eb6155a9b Mon Sep 17 00:00:00 2001 From: Korbinian Stoemmer Date: Tue, 9 Apr 2024 13:16:37 +0200 Subject: [PATCH] fix: create epp secret in epp-deployment ns (#534) * fix lint issues and revert changes to lint gha * create epp secret in same namespace as epp deployment * fix codeowners * use superlinter * fix workflow * include history in checkout * Revert "include history in checkout" This reverts commit 3d659ae3bcd0d78ced078a36462702ff6aa745c7. * Revert "fix workflow" This reverts commit 704f3ed744479d67a48265bf98279ced300a6613. * Revert "use superlinter" This reverts commit 646cee403e24a80317c7c1638c14389bbc29183c. * downgrade golangci-lint action * another linter bump * fix semantic-pr * apply code review comments * lint code * fix lint * fix nit --- .github/workflows/lint-conventional-prs.yml | 2 +- .github/workflows/lint-go.yml | 24 +++++++++-- .golangci.yaml | 1 + api/operator/v1alpha1/eventing_types_test.go | 2 +- hack/e2e/common/eventing/utils.go | 5 ++- hack/e2e/common/k8s.go | 2 +- .../operator/eventing/controller.go | 42 +++++++++---------- .../operator/eventing/controller_test.go | 10 ++--- .../controller/operator/eventing/eventmesh.go | 12 +++--- .../operator/eventing/eventmesh_test.go | 21 ++++++---- .../controller/integration_test.go | 13 +++--- 11 files changed, 76 insertions(+), 58 deletions(-) diff --git a/.github/workflows/lint-conventional-prs.yml b/.github/workflows/lint-conventional-prs.yml index e1ef6e39f..d1284e9f4 100644 --- a/.github/workflows/lint-conventional-prs.yml +++ b/.github/workflows/lint-conventional-prs.yml @@ -14,7 +14,7 @@ jobs: name: Check Title runs-on: ubuntu-latest steps: - - uses: amannn/action-semantic-pull-request@e9fabac35e210fea40ca5b14c0da95a099eff26f + - uses: amannn/action-semantic-pull-request@7ed886f1bd917d19cb9d6ce6c10e80e81fa31c39 env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} with: diff --git a/.github/workflows/lint-go.yml b/.github/workflows/lint-go.yml index 049a7eac7..0d703e5ee 100644 --- a/.github/workflows/lint-go.yml +++ b/.github/workflows/lint-go.yml @@ -1,9 +1,9 @@ -name: golangci-lint +name: Lint Go code on: pull_request: branches: - - main + - "main" - "release-**" paths-ignore: - "docs/**" @@ -20,8 +20,24 @@ permissions: contents: read # Optional: allow read access to pull request. Use with `only-new-issues` option. pull-requests: read + # Optional: Allow write access to checks to allow the action to annotate code in the PR. + checks: write jobs: - unit-test: + code-linter: name: "Run golangci-lint" - uses: kyma-project/eventing-tools/.github/workflows/lint-go-reusable.yml@main + runs-on: ubuntu-latest + + steps: + - name: Setup Go + uses: actions/setup-go@v5 + with: + go-version: 1.22 # This can be a specific version. E.g. 1.19 or 1.21.x + + - name: Checkout code + uses: actions/checkout@v4 + + - name: golangci-lint + uses: golangci/golangci-lint-action@v4.0.0 + with: + version: v1.55 diff --git a/.golangci.yaml b/.golangci.yaml index 30403d666..bb95d7e5c 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -210,6 +210,7 @@ linters-settings: - g # Ignore "g" as it is commonly used for gomega - a # Ignore as it is used in comparison functions - b # Ignore as it is used in comparison functions + - tc # Ignore as it is commonly used in table tests # Ignore "ok" variables that hold the bool return value of a type assertion. (defaults to false) ignore-type-assert-ok: true # Ignore "ok" variables that hold the bool return value of a map index. (defaults to false) diff --git a/api/operator/v1alpha1/eventing_types_test.go b/api/operator/v1alpha1/eventing_types_test.go index c69c78ba6..7bf182a47 100644 --- a/api/operator/v1alpha1/eventing_types_test.go +++ b/api/operator/v1alpha1/eventing_types_test.go @@ -1,9 +1,9 @@ package v1alpha1 import ( - "github.com/stretchr/testify/assert" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) diff --git a/hack/e2e/common/eventing/utils.go b/hack/e2e/common/eventing/utils.go index 797699a5e..8d1fea66a 100644 --- a/hack/e2e/common/eventing/utils.go +++ b/hack/e2e/common/eventing/utils.go @@ -44,11 +44,12 @@ func ExtractLegacyTypeFromSubscriptionV1Alpha2Type(eventVersion, eventType strin } // Assumption: The event type consists of at least 3 parts separated by the "." character. + minimalNumberOfSegments := 3 parts := strings.Split(eventType, ".") - if len(parts) < 3 { + if len(parts) < minimalNumberOfSegments { return "" } - parts = parts[len(parts)-3 : len(parts)-1] + parts = parts[len(parts)-minimalNumberOfSegments : len(parts)-1] return strings.Join(parts, ".") } diff --git a/hack/e2e/common/k8s.go b/hack/e2e/common/k8s.go index d353c4555..1b94e08c9 100644 --- a/hack/e2e/common/k8s.go +++ b/hack/e2e/common/k8s.go @@ -4,6 +4,7 @@ import ( "os" "path/filepath" + natsv1alpha1 "github.com/kyma-project/nats-manager/api/v1alpha1" "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/scheme" @@ -13,7 +14,6 @@ import ( eventingv1alpha2 "github.com/kyma-project/eventing-manager/api/eventing/v1alpha2" operatorv1alpha1 "github.com/kyma-project/eventing-manager/api/operator/v1alpha1" - natsv1alpha1 "github.com/kyma-project/nats-manager/api/v1alpha1" ) func GetK8sClients() (*kubernetes.Clientset, client.Client, *dynamic.DynamicClient, error) { diff --git a/internal/controller/operator/eventing/controller.go b/internal/controller/operator/eventing/controller.go index 6d514a60d..5843776ff 100644 --- a/internal/controller/operator/eventing/controller.go +++ b/internal/controller/operator/eventing/controller.go @@ -151,27 +151,27 @@ func NewReconciler( // RBAC permissions. //nolint:lll -//+kubebuilder:rbac:groups=operator.kyma-project.io,resources=eventings,verbs=get;list;watch;create;update;patch;delete -//+kubebuilder:rbac:groups=operator.kyma-project.io,resources=eventings/status,verbs=get;update;patch -//+kubebuilder:rbac:groups=operator.kyma-project.io,resources=eventings/finalizers,verbs=update -//+kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create;update;delete;patch -//+kubebuilder:rbac:groups="",resources=services,verbs=get;list;watch;update;patch;create;delete -//+kubebuilder:rbac:groups="rbac.authorization.k8s.io",resources=clusterroles,verbs=get;list;watch;update;patch;create;delete -//+kubebuilder:rbac:groups="rbac.authorization.k8s.io",resources=clusterrolebindings,verbs=get;list;watch;update;patch;create;delete -//+kubebuilder:rbac:groups="",resources=serviceaccounts,verbs=get;list;watch;update;patch;create;delete -//+kubebuilder:rbac:groups="autoscaling",resources=horizontalpodautoscalers,verbs=get;list;watch;create;update;patch;delete -//+kubebuilder:rbac:groups=security.istio.io,resources=customresourcedefinitions,verbs=get;list;watch -//+kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,verbs=get;list;watch -//+kubebuilder:rbac:groups="",resources=configmaps,verbs=get;list;watch;create;update;patch;delete -//+kubebuilder:rbac:groups=operator.kyma-project.io,resources=nats,verbs=get;list;watch -//+kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch;update;patch;create;delete -//+kubebuilder:rbac:groups="admissionregistration.k8s.io",resources=mutatingwebhookconfigurations,verbs=get;list;watch;update;patch;create;delete -//+kubebuilder:rbac:groups="admissionregistration.k8s.io",resources=validatingwebhookconfigurations,verbs=get;list;watch;update;patch;create;delete -//+kubebuilder:rbac:groups="applicationconnector.kyma-project.io",resources=applications,verbs=get;list;watch;update;patch;create;delete -//+kubebuilder:rbac:groups=gateway.kyma-project.io,resources=apirules,verbs=get;list;watch;create;update;patch;delete -//+kubebuilder:rbac:groups="eventing.kyma-project.io",resources=subscriptions,verbs=get;list;watch;update;patch;create;delete +// +kubebuilder:rbac:groups=operator.kyma-project.io,resources=eventings,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=operator.kyma-project.io,resources=eventings/status,verbs=get;update;patch +// +kubebuilder:rbac:groups=operator.kyma-project.io,resources=eventings/finalizers,verbs=update +// +kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create;update;delete;patch +// +kubebuilder:rbac:groups="",resources=services,verbs=get;list;watch;update;patch;create;delete +// +kubebuilder:rbac:groups="rbac.authorization.k8s.io",resources=clusterroles,verbs=get;list;watch;update;patch;create;delete +// +kubebuilder:rbac:groups="rbac.authorization.k8s.io",resources=clusterrolebindings,verbs=get;list;watch;update;patch;create;delete +// +kubebuilder:rbac:groups="",resources=serviceaccounts,verbs=get;list;watch;update;patch;create;delete +// +kubebuilder:rbac:groups="autoscaling",resources=horizontalpodautoscalers,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=security.istio.io,resources=customresourcedefinitions,verbs=get;list;watch +// +kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,verbs=get;list;watch +// +kubebuilder:rbac:groups="",resources=configmaps,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=operator.kyma-project.io,resources=nats,verbs=get;list;watch +// +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch;update;patch;create;delete +// +kubebuilder:rbac:groups="admissionregistration.k8s.io",resources=mutatingwebhookconfigurations,verbs=get;list;watch;update;patch;create;delete +// +kubebuilder:rbac:groups="admissionregistration.k8s.io",resources=validatingwebhookconfigurations,verbs=get;list;watch;update;patch;create;delete +// +kubebuilder:rbac:groups="applicationconnector.kyma-project.io",resources=applications,verbs=get;list;watch;update;patch;create;delete +// +kubebuilder:rbac:groups=gateway.kyma-project.io,resources=apirules,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups="eventing.kyma-project.io",resources=subscriptions,verbs=get;list;watch;update;patch;create;delete // +kubebuilder:rbac:groups=eventing.kyma-project.io,resources=subscriptions/status,verbs=get;update;patch -//+kubebuilder:rbac:groups=security.istio.io,resources=peerauthentications,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=security.istio.io,resources=peerauthentications,verbs=get;list;watch;create;update;patch;delete // Generate required RBAC to emit kubernetes events in the controller. // +kubebuilder:rbac:groups="",resources=events,verbs=create;patch @@ -671,7 +671,7 @@ func (r *Reconciler) reconcileEventMeshBackend(ctx context.Context, eventing *op // Start the EventMesh subscription controller err = r.reconcileEventMeshSubManager(ctx, eventing, eventMeshSecret) if err != nil { - // In case the the error is caused by a malformatted secret, we want to set the status to warning, + // In case the error is caused by a malformatted secret, we want to set the status to warning, // to indicate the requirement of user interaction. if IsMalformattedSecretErr(err) { return kctrl.Result{}, r.syncSubManagerStatusWithNATSState(ctx, operatorv1alpha1.StateWarning, eventing, diff --git a/internal/controller/operator/eventing/controller_test.go b/internal/controller/operator/eventing/controller_test.go index a7e720730..f9c9728d6 100644 --- a/internal/controller/operator/eventing/controller_test.go +++ b/internal/controller/operator/eventing/controller_test.go @@ -431,11 +431,11 @@ func Test_stopNatsCRWatch(t *testing.T) { }, } - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { + for _, testcase := range testCases { + t.Run(testcase.name, func(t *testing.T) { // given testEnv := NewMockedUnitTestEnvironment(t) - testEnv.Reconciler.natsCRWatchStarted = tc.natsCRWatchStarted + testEnv.Reconciler.natsCRWatchStarted = testcase.natsCRWatchStarted // Create a fake Watcher natsWatcher := new(watchermocks.Watcher) @@ -451,8 +451,8 @@ func Test_stopNatsCRWatch(t *testing.T) { testEnv.Reconciler.stopNATSCRWatch(eventing) // Check the results - require.Nil(t, tc.watchNatsWatcher) - require.False(t, tc.natsCRWatchStarted) + require.Nil(t, testcase.watchNatsWatcher) + require.False(t, testcase.natsCRWatchStarted) }) } } diff --git a/internal/controller/operator/eventing/eventmesh.go b/internal/controller/operator/eventing/eventmesh.go index 87d8702e8..e2c10e4ff 100644 --- a/internal/controller/operator/eventing/eventmesh.go +++ b/internal/controller/operator/eventing/eventmesh.go @@ -31,7 +31,7 @@ const ( EMSecretMessagingMissing = "messaging is missing from EM secret" EMSecretNamespaceMissing = "namespace is missing from EM secret" EventMeshSecretMissingMessage = "The specified EventMesh secret is not found. Please provide an existing secret." - EventMeshSecretMalformatted = "The EventMesh secret data is not formatted properly." + EventMeshSecretMalformatted = "The EventMesh secret data is not formatted properly." //nolint:gosec // just an error message ) var ( @@ -66,7 +66,7 @@ func (r *Reconciler) reconcileEventMeshSubManager(ctx context.Context, eventing } // CreateOrUpdate deployment for publisher proxy secret - secretForPublisher, err := r.SyncPublisherProxySecret(ctx, eventMeshSecret) + secretForPublisher, err := r.SyncPublisherProxySecret(ctx, eventing.Namespace, eventMeshSecret) if err != nil { return fmt.Errorf("failed to sync Publisher Proxy secret: %w", err) } @@ -199,8 +199,8 @@ func (r *Reconciler) stopEventMeshSubManager(runCleanup bool, log *zap.SugaredLo return nil } -func (r *Reconciler) SyncPublisherProxySecret(ctx context.Context, secret *kcorev1.Secret) (*kcorev1.Secret, error) { - desiredSecret, err := getSecretForPublisher(secret) +func (r *Reconciler) SyncPublisherProxySecret(ctx context.Context, eventingNS string, secret *kcorev1.Secret) (*kcorev1.Secret, error) { + desiredSecret, err := getSecretForPublisher(secret, eventingNS) if err != nil { return nil, fmt.Errorf("invalid secret for Event Publisher: %w", err) } @@ -321,8 +321,8 @@ func newSecret(name, namespace string) *kcorev1.Secret { } } -func getSecretForPublisher(eventMeshSecret *kcorev1.Secret) (*kcorev1.Secret, error) { - secret := newSecret(eventing.PublisherName, eventMeshSecret.Namespace) +func getSecretForPublisher(eventMeshSecret *kcorev1.Secret, ns string) (*kcorev1.Secret, error) { + secret := newSecret(eventing.PublisherName, ns) secret.Labels = map[string]string{ label.KeyName: label.ValueEventingPublisherProxy, diff --git a/internal/controller/operator/eventing/eventmesh_test.go b/internal/controller/operator/eventing/eventmesh_test.go index 40a352fa3..cf0e07f21 100644 --- a/internal/controller/operator/eventing/eventmesh_test.go +++ b/internal/controller/operator/eventing/eventmesh_test.go @@ -537,6 +537,7 @@ func Test_stopEventMeshSubManager(t *testing.T) { // TestGetSecretForPublisher verifies the successful and failing retrieval // of secrets. func Test_GetSecretForPublisher(t *testing.T) { + eventingNS := "eventingns" secretFor := func(message, namespace []byte) *kcorev1.Secret { secret := &kcorev1.Secret{ ObjectMeta: kmetav1.ObjectMeta{ @@ -621,7 +622,7 @@ func Test_GetSecretForPublisher(t *testing.T) { }, ObjectMeta: kmetav1.ObjectMeta{ Name: eventing.PublisherName, - Namespace: "test-namespace", + Namespace: eventingNS, Labels: map[string]string{ label.KeyName: label.ValueEventingPublisherProxy, }, @@ -694,19 +695,19 @@ func Test_GetSecretForPublisher(t *testing.T) { }, } - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - publisherSecret := secretFor(tc.messagingData, tc.namespaceData) + for _, testcase := range testCases { + t.Run(testcase.name, func(t *testing.T) { + publisherSecret := secretFor(testcase.messagingData, testcase.namespaceData) - gotPublisherSecret, err := getSecretForPublisher(publisherSecret) - if tc.expectedError != nil { + gotPublisherSecret, err := getSecretForPublisher(publisherSecret, eventingNS) + if testcase.expectedError != nil { require.Error(t, err) require.ErrorIs(t, err, ErrEventMeshSecretMalformatted) - require.ErrorContains(t, err, tc.expectedError.Error()) + require.ErrorContains(t, err, testcase.expectedError.Error()) return } require.NoError(t, err) - require.Equal(t, tc.expectedSecret, gotPublisherSecret, "invalid publisher secret") + require.Equal(t, testcase.expectedSecret, gotPublisherSecret, "invalid publisher secret") }) } } @@ -932,7 +933,7 @@ func Test_SyncPublisherProxySecret(t *testing.T) { } // when - _, err := r.SyncPublisherProxySecret(context.Background(), testcase.givenSecret) + _, err := r.SyncPublisherProxySecret(context.Background(), "eventingns", testcase.givenSecret) // then if testcase.wantErr { @@ -1120,6 +1121,8 @@ func Test_syncOauth2ClientIDAndSecret(t *testing.T) { // Test_IsMalfromattedSecret verifies that the function IsMalformattedSecretErr asses correctly // if a given error is a malformatted secret error or not. + +//nolint:goerr113 // test for error checking function func Test_IsMalfromattedSecret(t *testing.T) { testCases := []struct { name string diff --git a/internal/controller/operator/eventing/integrationtests/controller/integration_test.go b/internal/controller/operator/eventing/integrationtests/controller/integration_test.go index cd52182d3..e284913ec 100644 --- a/internal/controller/operator/eventing/integrationtests/controller/integration_test.go +++ b/internal/controller/operator/eventing/integrationtests/controller/integration_test.go @@ -722,6 +722,7 @@ func Test_CreateEventingCR_EventMesh(t *testing.T) { } func TestUpdateEventingCRFromEmptyToNonEmptyBackend(t *testing.T) { + t.Parallel() tests := []struct { name string givenBackendTypeToUse operatorv1alpha1.BackendType @@ -771,9 +772,7 @@ func TestUpdateEventingCRFromEmptyToNonEmptyBackend(t *testing.T) { t.Parallel() g := gomega.NewWithT(t) - ///// // Create the Eventing CR with an empty backend - ///// eventingCR := utils.NewEventingCR(utils.WithEmptyBackend()) namespace := eventingCR.Namespace @@ -786,9 +785,7 @@ func TestUpdateEventingCRFromEmptyToNonEmptyBackend(t *testing.T) { testEnvironment.GetEventingAssert(g, eventingCR).Should(test.wantMatchesBeforeUpdate) - ///// // Update the Eventing CR with a non-empty backend - ///// eventingCR, err := testEnvironment.GetEventingFromK8s(eventingCR.Name, namespace) g.Expect(err).ShouldNot(gomega.HaveOccurred()) @@ -1116,8 +1113,8 @@ func Test_HandlingMalformedEventMeshSecret(t *testing.T) { }, } - for _, tc := range testcases { - t.Run(tc.name, func(t *testing.T) { + for _, testcase := range testcases { + t.Run(testcase.name, func(t *testing.T) { // Given: // We need to mock the deployment readiness check. eventingcontroller.IsDeploymentReady = func(deployment *kappsv1.Deployment) bool { @@ -1149,7 +1146,7 @@ func Test_HandlingMalformedEventMeshSecret(t *testing.T) { Name: name, Namespace: namespace, }, - Data: tc.givenData, + Data: testcase.givenData, Type: "Opaque", } // Finally, we can create the EventMesh Secret on the cluster. @@ -1162,7 +1159,7 @@ func Test_HandlingMalformedEventMeshSecret(t *testing.T) { // Then: // Check if the EventingCR status has the expected status, caused by the EventMesh Secret. g := gomega.NewWithT(t) - testEnvironment.GetEventingAssert(g, givenEventingCR).Should(tc.wantMatcher) + testEnvironment.GetEventingAssert(g, givenEventingCR).Should(testcase.wantMatcher) }) } }