diff --git a/.github/workflows/lint-go.yml b/.github/workflows/lint-go.yml index de3f559ab..1c5763222 100644 --- a/.github/workflows/lint-go.yml +++ b/.github/workflows/lint-go.yml @@ -18,4 +18,4 @@ permissions: jobs: unit-test: name: "Run golangci-lint" - uses: kyma-project/eventing-tools/.github/workflows/lint-reusable.yml@main + uses: kyma-project/eventing-tools/.github/workflows/lint-go-reusable.yml@main diff --git a/internal/controller/operator/eventing/controller.go b/internal/controller/operator/eventing/controller.go index 7db30dbdf..316d4befd 100644 --- a/internal/controller/operator/eventing/controller.go +++ b/internal/controller/operator/eventing/controller.go @@ -669,6 +669,12 @@ 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, + // to indicate the requirement of user interaction. + if IsMalformattedSecretErr(err) { + return kctrl.Result{}, r.syncSubManagerStatusWithNATSState(ctx, operatorv1alpha1.StateWarning, eventing, + ErrEventMeshSecretMalformatted, log) + } return kctrl.Result{}, r.syncStatusWithSubscriptionManagerErr(ctx, eventing, err, log) } eventing.Status.SetSubscriptionManagerReadyConditionToTrue() diff --git a/internal/controller/operator/eventing/controller_test.go b/internal/controller/operator/eventing/controller_test.go index 3a43b3d4d..a7e720730 100644 --- a/internal/controller/operator/eventing/controller_test.go +++ b/internal/controller/operator/eventing/controller_test.go @@ -432,11 +432,10 @@ func Test_stopNatsCRWatch(t *testing.T) { } for _, tc := range testCases { - testcase := tc - t.Run(testcase.name, func(t *testing.T) { + t.Run(tc.name, func(t *testing.T) { // given testEnv := NewMockedUnitTestEnvironment(t) - testEnv.Reconciler.natsCRWatchStarted = testcase.natsCRWatchStarted + testEnv.Reconciler.natsCRWatchStarted = tc.natsCRWatchStarted // Create a fake Watcher natsWatcher := new(watchermocks.Watcher) @@ -452,8 +451,8 @@ func Test_stopNatsCRWatch(t *testing.T) { testEnv.Reconciler.stopNATSCRWatch(eventing) // Check the results - require.Equal(t, nil, testcase.watchNatsWatcher) - require.False(t, testcase.natsCRWatchStarted) + require.Nil(t, tc.watchNatsWatcher) + require.False(t, tc.natsCRWatchStarted) }) } } diff --git a/internal/controller/operator/eventing/eventmesh.go b/internal/controller/operator/eventing/eventmesh.go index b26ef9c62..87d8702e8 100644 --- a/internal/controller/operator/eventing/eventmesh.go +++ b/internal/controller/operator/eventing/eventmesh.go @@ -28,13 +28,17 @@ const ( secretKeyTokenURL = "token_url" secretKeyCertsURL = "certs_url" + 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." ) var ( - ErrEMSecretMessagingMissing = errors.New("messaging is missing from EM secret") - ErrEMSecretNamespaceMissing = errors.New("namespace is missing from EM secret") - ErrEventMeshSecretMissing = errors.New(EventMeshSecretMissingMessage) + ErrEMSecretMessagingMissing = errors.New(EMSecretMessagingMissing) + ErrEMSecretNamespaceMissing = errors.New(EMSecretNamespaceMissing) + ErrEventMeshSecretMissing = errors.New(EventMeshSecretMissingMessage) + ErrEventMeshSecretMalformatted = errors.New(EventMeshSecretMalformatted) ) type oauth2Credentials struct { @@ -44,6 +48,16 @@ type oauth2Credentials struct { certsURL []byte } +func newMalformattedSecretErr(e string) error { + // The new error is handled as a string (%s), the malformation error is wrapped as an Err. + return fmt.Errorf("%s: %w", e, ErrEventMeshSecretMalformatted) +} + +// IsMalformattedSecretErr checks if the error is of type ErrEventMeshSecretMalformatted. +func IsMalformattedSecretErr(err error) bool { + return errors.Is(err, ErrEventMeshSecretMalformatted) +} + func (r *Reconciler) reconcileEventMeshSubManager(ctx context.Context, eventing *v1alpha1.Eventing, eventMeshSecret *kcorev1.Secret) error { // gets oauth2ClientID and secret and stops the EventMesh subscription manager if changed err := r.syncOauth2ClientIDAndSecret(ctx, eventing) @@ -314,38 +328,38 @@ func getSecretForPublisher(eventMeshSecret *kcorev1.Secret) (*kcorev1.Secret, er label.KeyName: label.ValueEventingPublisherProxy, } - if _, ok := eventMeshSecret.Data["messaging"]; !ok { - return nil, ErrEMSecretMessagingMissing - } - messagingBytes := eventMeshSecret.Data["messaging"] - if _, ok := eventMeshSecret.Data["namespace"]; !ok { - return nil, ErrEMSecretNamespaceMissing + return nil, newMalformattedSecretErr(EMSecretNamespaceMissing) } namespaceBytes := eventMeshSecret.Data["namespace"] + if _, ok := eventMeshSecret.Data["messaging"]; !ok { + return nil, newMalformattedSecretErr(EMSecretMessagingMissing) + } + messagingBytes := eventMeshSecret.Data["messaging"] + var messages []Message err := json.Unmarshal(messagingBytes, &messages) if err != nil { - return nil, err + return nil, newMalformattedSecretErr(err.Error()) } for _, msg := range messages { if msg.Broker.BrokerType == "saprestmgw" { if len(msg.OA2.ClientID) == 0 { - return nil, errors.New("client ID is missing") + return nil, newMalformattedSecretErr("client ID is missing") } if len(msg.OA2.ClientSecret) == 0 { - return nil, errors.New("client secret is missing") + return nil, newMalformattedSecretErr("client secret is missing") } if len(msg.OA2.TokenEndpoint) == 0 { - return nil, errors.New("tokenendpoint is missing") + return nil, newMalformattedSecretErr("tokenendpoint is missing") } if len(msg.OA2.GrantType) == 0 { - return nil, errors.New("granttype is missing") + return nil, newMalformattedSecretErr("granttype is missing") } if len(msg.URI) == 0 { - return nil, errors.New("publish URL is missing") + return nil, newMalformattedSecretErr("publish URL is missing") } secret.StringData = getSecretStringData(msg.OA2.ClientID, msg.OA2.ClientSecret, msg.OA2.TokenEndpoint, msg.OA2.GrantType, msg.URI, string(namespaceBytes)) diff --git a/internal/controller/operator/eventing/eventmesh_test.go b/internal/controller/operator/eventing/eventmesh_test.go index 02bb8b45f..40a352fa3 100644 --- a/internal/controller/operator/eventing/eventmesh_test.go +++ b/internal/controller/operator/eventing/eventmesh_test.go @@ -695,18 +695,18 @@ func Test_GetSecretForPublisher(t *testing.T) { } for _, tc := range testCases { - testcase := tc - t.Run(testcase.name, func(t *testing.T) { - publisherSecret := secretFor(testcase.messagingData, testcase.namespaceData) + t.Run(tc.name, func(t *testing.T) { + publisherSecret := secretFor(tc.messagingData, tc.namespaceData) gotPublisherSecret, err := getSecretForPublisher(publisherSecret) - if testcase.expectedError != nil { + if tc.expectedError != nil { require.Error(t, err) - require.ErrorContains(t, err, testcase.expectedError.Error()) + require.ErrorIs(t, err, ErrEventMeshSecretMalformatted) + require.ErrorContains(t, err, tc.expectedError.Error()) return } require.NoError(t, err) - require.Equal(t, testcase.expectedSecret, gotPublisherSecret, "invalid publisher secret") + require.Equal(t, tc.expectedSecret, gotPublisherSecret, "invalid publisher secret") }) } } @@ -1117,3 +1117,37 @@ 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. +func Test_IsMalfromattedSecret(t *testing.T) { + testCases := []struct { + name string + givenErr error + wantResult bool + }{ + { + name: "should return true when an error is an ErrMalformedSecret", + givenErr: ErrEventMeshSecretMalformatted, + wantResult: true, + }, { + name: "should return true when an error is a wrapped ErrMalformedSecret", + givenErr: newMalformattedSecretErr("this error will wrap ErrMalformedSecret"), + wantResult: true, + }, { + name: "should return false when an error is not an ErrMalformedSecret", + givenErr: fmt.Errorf("this is not a malformed secret error"), + wantResult: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // When: + result := IsMalformattedSecretErr(tc.givenErr) + + // Then: + require.Equal(t, tc.wantResult, result) + }) + } +} diff --git a/internal/controller/operator/eventing/integrationtests/controller/integration_test.go b/internal/controller/operator/eventing/integrationtests/controller/integration_test.go index 9c1239e95..c59e07bdd 100644 --- a/internal/controller/operator/eventing/integrationtests/controller/integration_test.go +++ b/internal/controller/operator/eventing/integrationtests/controller/integration_test.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" "os" + "strings" "testing" natsv1alpha1 "github.com/kyma-project/nats-manager/api/v1alpha1" @@ -14,6 +15,7 @@ import ( "github.com/pkg/errors" "github.com/stretchr/testify/require" kappsv1 "k8s.io/api/apps/v1" + kcorev1 "k8s.io/api/core/v1" kapiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" kmetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -867,6 +869,201 @@ func Test_DeleteEventingCR(t *testing.T) { } } +func Test_HandlingMalformedEventMeshSecret(t *testing.T) { + testcases := []struct { + name string + givenData map[string][]byte + wantMatcher gomegatypes.GomegaMatcher + }{ + { + name: "EventingCR should have the `ready` status when EventMesh secret is valid", + givenData: map[string][]byte{ + "management": []byte("foo"), + "messaging": []byte(`[ + { + "broker": { + "type": "bar" + }, + "oa2": { + "clientid": "foo", + "clientsecret": "foo", + "granttype": "client_credentials", + "tokenendpoint": "bar" + }, + "protocol": [ + "amqp10ws" + ], + "uri": "foo" + }, + { + "broker": { + "type": "foo" + }, + "oa2": { + "clientid": "bar", + "clientsecret": "bar", + "granttype": "client_credentials", + "tokenendpoint": "foo" + }, + "protocol": [ + "bar" + ], + "uri": "bar" + }, + { + "broker": { + "type": "foo" + }, + "oa2": { + "clientid": "foo", + "clientsecret": "bar", + "granttype": "client_credentials", + "tokenendpoint": "foo" + }, + "protocol": [ + "httprest" + ], + "uri": "bar" + } + ]`), + "namespace": []byte("bar"), + "serviceinstanceid": []byte("foo"), + "xsappname": []byte("bar"), + }, + wantMatcher: gomega.And( + matchers.HaveStatusReady(), + ), + }, + { + name: "EventingCR should be have the `warning` status when EventMesh secret data is empty", + givenData: map[string][]byte{}, + wantMatcher: gomega.And( + matchers.HaveStatusWarning(), + ), + }, + { + name: "EventingCR should have the `warning` status when EventMesh secret data misses the `namespace` key", + givenData: map[string][]byte{ + "management": []byte("foo"), + "messaging": []byte(`[ + { + "broker": { + "type": "bar" + }, + "oa2": { + "clientid": "foo", + "clientsecret": "foo", + "granttype": "client_credentials", + "tokenendpoint": "bar" + }, + "protocol": [ + "amqp10ws" + ], + "uri": "foo" + }, + { + "broker": { + "type": "foo" + }, + "oa2": { + "clientid": "bar", + "clientsecret": "bar", + "granttype": "client_credentials", + "tokenendpoint": "foo" + }, + "protocol": [ + "bar" + ], + "uri": "bar" + }, + { + "broker": { + "type": "foo" + }, + "oa2": { + "clientid": "foo", + "clientsecret": "bar", + "granttype": "client_credentials", + "tokenendpoint": "foo" + }, + "protocol": [ + "httprest" + ], + "uri": "bar" + } + ]`), + "serviceinstanceid": []byte("foo"), + "xsappname": []byte("bar"), + }, + wantMatcher: gomega.And( + matchers.HaveStatusWarning(), + ), + }, + { + name: "EventingCR should have the `warning` status when EventMesh secret data misses the `messaging` key", + givenData: map[string][]byte{ + "management": []byte("foo"), + "serviceinstanceid": []byte("foo"), + "xsappname": []byte("bar"), + "namespace": []byte("bar"), + }, + wantMatcher: gomega.And( + matchers.HaveStatusWarning(), + ), + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + // Given: + // We need to mock the deployment readiness check. + eventingcontroller.IsDeploymentReady = func(deployment *kappsv1.Deployment) bool { + return true + } + + // Create an eventing CR with EventMesh backend. + givenEventingCR := utils.NewEventingCR( + utils.WithEventMeshBackend("test-secret-name2"), + utils.WithEventingPublisherData(2, 2, "199m", "99Mi", "399m", "199Mi"), + utils.WithEventingEventTypePrefix("test-prefix"), + utils.WithEventingDomain(utils.Domain), + ) + + // Create an unique namespace for this test run. + givenNamespace := givenEventingCR.Namespace + testEnvironment.EnsureNamespaceCreation(t, givenNamespace) + + // Create an eventing-webhook-auth Secret. + testEnvironment.EnsureOAuthSecretCreated(t, givenEventingCR) + + // Create EventMesh secret. This is the crucial part of the test. + // First we need to extract the expected Secret name and namespace from the Eventing CR. + a := strings.Split(givenEventingCR.Spec.Backend.Config.EventMeshSecret, "/") + name, namespace := a[1], a[0] + // Now we can assemble the EventMesh Secret with the given data. + secret := &kcorev1.Secret{ + ObjectMeta: kmetav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Data: tc.givenData, + Type: "Opaque", + } + // Finally, we can create the EventMesh Secret on the cluster. + testEnvironment.EnsureK8sResourceCreated(t, secret) + + // When: + // Create the Eventing CR on the cluster. + testEnvironment.EnsureK8sResourceCreated(t, givenEventingCR) + + // 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) + }) + } +} + func Test_WatcherNATSResource(t *testing.T) { t.Parallel() testCases := []struct {