Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Malformatted EventMesh Secret causes warning status #520

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/lint-go.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 6 additions & 0 deletions internal/controller/operator/eventing/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
9 changes: 4 additions & 5 deletions internal/controller/operator/eventing/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
mfaizanse marked this conversation as resolved.
Show resolved Hide resolved
})
}
}
44 changes: 29 additions & 15 deletions internal/controller/operator/eventing/eventmesh.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
Expand Down Expand Up @@ -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))
Expand Down
46 changes: 40 additions & 6 deletions internal/controller/operator/eventing/eventmesh_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
friedrichwilken marked this conversation as resolved.
Show resolved Hide resolved
return
}
require.NoError(t, err)
require.Equal(t, testcase.expectedSecret, gotPublisherSecret, "invalid publisher secret")
require.Equal(t, tc.expectedSecret, gotPublisherSecret, "invalid publisher secret")
})
}
}
Expand Down Expand Up @@ -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)
})
}
}
Loading
Loading