Skip to content

Commit

Permalink
fix: create epp secret in epp-deployment ns (kyma-project#534)
Browse files Browse the repository at this point in the history
* 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 3d659ae.

* Revert "fix workflow"

This reverts commit 704f3ed.

* Revert "use superlinter"

This reverts commit 646cee4.

* downgrade golangci-lint action

* another linter bump

* fix semantic-pr

* apply code review comments

* lint code

* fix lint

* fix nit
  • Loading branch information
k15r authored Apr 9, 2024
1 parent c542473 commit 8d62ae0
Show file tree
Hide file tree
Showing 11 changed files with 76 additions and 58 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/lint-conventional-prs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
24 changes: 20 additions & 4 deletions .github/workflows/lint-go.yml
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
name: golangci-lint
name: Lint Go code

on:
pull_request:
branches:
- main
- "main"
- "release-**"
paths-ignore:
- "docs/**"
Expand All @@ -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/[email protected]
with:
version: v1.55
1 change: 1 addition & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion api/operator/v1alpha1/eventing_types_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package v1alpha1

import (
"github.com/stretchr/testify/assert"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

Expand Down
5 changes: 3 additions & 2 deletions hack/e2e/common/eventing/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, ".")
}

Expand Down
2 changes: 1 addition & 1 deletion hack/e2e/common/k8s.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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) {
Expand Down
42 changes: 21 additions & 21 deletions internal/controller/operator/eventing/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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,
Expand Down
10 changes: 5 additions & 5 deletions internal/controller/operator/eventing/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
})
}
}
12 changes: 6 additions & 6 deletions internal/controller/operator/eventing/eventmesh.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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,
Expand Down
21 changes: 12 additions & 9 deletions internal/controller/operator/eventing/eventmesh_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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")
})
}
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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())
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
Expand All @@ -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)
})
}
}
Expand Down

0 comments on commit 8d62ae0

Please sign in to comment.