reconcile configmap (#35)

* reconcile configmap

.github/workflows/main.yml

@@ -0,0 +1,17 @@
+name: main
+on:
+  push:
+    branches:
+    - main
+jobs:
+  cover:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v4
+    - uses: actions/setup-go@v5
+      with:
+        go-version: '1.22'
+    - run: make test
+    - uses: shogo82148/actions-goveralls@v1
+      with:
+        path-to-profile: cover.out

.github/workflows/pull-request.yml

@@ -17,22 +17,14 @@ jobs:
   make-test-cover:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/cache@v3
+    - uses: actions/checkout@v4
+    - uses: actions/setup-go@v5
       with:
-        path: |
-          ~/.cache/go-build
-          ~/go/pkg/mod
-        key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}
-        restore-keys: |
-          ${{ runner.os }}-go-
-    - uses: actions/setup-go@v4
-    - uses: actions/checkout@v3
-    - run: go mod download
+        go-version: '1.22'
     - run: make test
-    - run: go install github.com/mattn/[email protected]
-    - run: goveralls -coverprofile=cover.out -service=github
-      env:
-        COVERALLS_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+    - uses: shogo82148/actions-goveralls@v1
+      with:
+        path-to-profile: cover.out
 
   docker-build:
     runs-on: ubuntu-latest

.golangci.yml

@@ -25,7 +25,6 @@ linters:
   enable:
     - dupl
     - errcheck
-    - exportloopref
     - ginkgolinter
     - goconst
     - gocyclo

Makefile

@@ -70,7 +70,7 @@ test-e2e:
 
 .PHONY: lint
 lint: golangci-lint ## Run golangci-lint linter
-	$(GOLANGCI_LINT) run
+	$(GOLANGCI_LINT) run -v
 
 .PHONY: lint-fix
 lint-fix: golangci-lint ## Run golangci-lint linter and perform fixes
@@ -172,7 +172,7 @@ MOCKGEN ?= $(LOCALBIN)/mockgen
 KUSTOMIZE_VERSION ?= v5.4.2
 CONTROLLER_TOOLS_VERSION ?= v0.15.0
 ENVTEST_VERSION ?= release-0.18
-GOLANGCI_LINT_VERSION ?= v1.59.1
+GOLANGCI_LINT_VERSION ?= v1.60.2
 MOCKGEN_VERSION ?= v0.4.0
 
 .PHONY: kustomize

controllers/configmapcontroller/configmap_controller.go

@@ -22,11 +22,14 @@ import (
 	"time"
 
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/api/errors"
+	networkingv1 "k8s.io/api/networking/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/util/retry"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
+	"github.com/kuoss/ingress-annotator/pkg/model"
 	"github.com/kuoss/ingress-annotator/pkg/rulesstore"
 )
 
@@ -60,7 +63,7 @@ func (r *ConfigMapReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 	// Fetch the ConfigMap resource
 	var cm corev1.ConfigMap
 	if err := r.Get(ctx, r.NN, &cm); err != nil {
-		if errors.IsNotFound(err) {
+		if apierrors.IsNotFound(err) {
 			logger.Error(err, "ConfigMap %s not found, will retry after delay", r.NN)
 			return ctrl.Result{RequeueAfter: 30 * time.Second}, nil
 		}
@@ -78,6 +81,42 @@ func (r *ConfigMapReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 	newRules := r.RulesStore.GetRules()
 	logger.Info("Rules updated", "newRules", newRules)
 
+	if err := r.annotateAllIngresses(ctx); err != nil {
+		return ctrl.Result{}, fmt.Errorf("failed to annotateAllIngresses: %w", err)
+	}
+
 	logger.Info("Successfully reconciled ConfigMap")
 	return ctrl.Result{}, nil
 }
+
+func (r *ConfigMapReconciler) annotateAllIngresses(ctx context.Context) error {
+	var ingressList networkingv1.IngressList
+
+	if err := r.List(ctx, &ingressList); err != nil {
+		return fmt.Errorf("failed to list ingresses: %w", err)
+	}
+
+	for _, ing := range ingressList.Items {
+		if err := r.annotateIngress(ctx, ing); err != nil {
+			return fmt.Errorf("failed to annotateIngress: %w", err)
+		}
+	}
+
+	return nil
+}
+
+func (r *ConfigMapReconciler) annotateIngress(ctx context.Context, ing networkingv1.Ingress) error {
+	return retry.RetryOnConflict(retry.DefaultRetry, func() error {
+		if err := r.Get(ctx, client.ObjectKey{Name: ing.Name, Namespace: ing.Namespace}, &ing); err != nil {
+			return fmt.Errorf("failed to get ingress %s/%s: %w", ing.Namespace, ing.Name, err)
+		}
+		ing.SetAnnotations(map[string]string{model.ReconcileKey: "true"})
+		if err := r.Update(ctx, &ing); err != nil {
+			if apierrors.IsConflict(err) {
+				return err
+			}
+			return fmt.Errorf("failed to update ingress %s/%s: %w", ing.Namespace, ing.Name, err)
+		}
+		return nil
+	})
+}

controllers/configmapcontroller/configmap_controller_test.go

@@ -9,6 +9,8 @@ import (
 	"github.com/stretchr/testify/assert"
 	"go.uber.org/mock/gomock"
 	corev1 "k8s.io/api/core/v1"
+	networkingv1 "k8s.io/api/networking/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
 
@@ -70,24 +72,36 @@ func TestConfigMapReconciler_Reconcile(t *testing.T) {
 		wantError  string
 	}{
 		{
-			name:       "Error on ConfigMap Get should return appropriate error and requeue",
-			clientOpts: &fakeclient.ClientOpts{GetError: true}, // Simulate a Get error on Get
+			name:       "Requeue on ConfigMap Get error",
+			clientOpts: &fakeclient.ClientOpts{GetError: true},
 			cm:         createConfigMap("default", "ingress-annotator", ""),
+			newCM:      createConfigMap("default", "ingress-annotator", "rule1:\n  key1: value1"),
 			nn:         types.NamespacedName{Namespace: "default", Name: "ingress-annotator"},
 			requestNN:  types.NamespacedName{Namespace: "default", Name: "ingress-annotator"},
-			wantError:  "failed to get ConfigMap: mocked GetError",
 			want:       ctrl.Result{RequeueAfter: 30 * time.Second},
+			wantError:  "failed to get ConfigMap: mocked GetError",
 		},
 		{
-			name:       "ConfigMap not found should requeue after 30 seconds",
-			clientOpts: &fakeclient.ClientOpts{GetNotFoundError: true}, // Simulate a NotFound error on Get
+			name:       "Requeue when ConfigMap not found",
+			clientOpts: &fakeclient.ClientOpts{GetNotFoundError: true},
 			cm:         createConfigMap("default", "ingress-annotator", ""),
+			newCM:      createConfigMap("default", "ingress-annotator", "rule1:\n  key1: value1"),
 			nn:         types.NamespacedName{Namespace: "default", Name: "ingress-annotator"},
 			requestNN:  types.NamespacedName{Namespace: "default", Name: "ingress-annotator"},
 			want:       ctrl.Result{RequeueAfter: 30 * time.Second},
 		},
 		{
-			name:      "Invalid ConfigMap data should return unmarshalling error",
+			name:       "Error during Ingress list should result in requeue",
+			clientOpts: &fakeclient.ClientOpts{ListError: true},
+			cm:         createConfigMap("default", "ingress-annotator", ""),
+			newCM:      createConfigMap("default", "ingress-annotator", "rule1:\n  key1: value1"),
+			nn:         types.NamespacedName{Namespace: "default", Name: "ingress-annotator"},
+			requestNN:  types.NamespacedName{Namespace: "default", Name: "ingress-annotator"},
+			want:       ctrl.Result{},
+			wantError:  "failed to annotateAllIngresses: failed to list ingresses: mocked ListError",
+		},
+		{
+			name:      "Unmarshal error on invalid ConfigMap data",
 			cm:        createConfigMap("default", "ingress-annotator", "rule1:\n  key1: value1"),
 			newCM:     createConfigMap("default", "ingress-annotator", "invalid rules"),
 			nn:        types.NamespacedName{Namespace: "default", Name: "ingress-annotator"},
@@ -96,22 +110,22 @@ func TestConfigMapReconciler_Reconcile(t *testing.T) {
 			wantError: "failed to update rules in rules store: failed to extract rules from configMap: failed to unmarshal rules: yaml: unmarshal errors:\n  line 1: cannot unmarshal !!str `invalid...` into model.Rules",
 		},
 		{
-			name:      "Valid ConfigMap but no change should not requeue or return an error",
+			name:      "No requeue when ConfigMap has no changes",
 			cm:        createConfigMap("default", "ingress-annotator", "rule1:\n  key1: value1"),
 			newCM:     createConfigMap("default", "ingress-annotator", "rule1:\n  key1: value1"),
 			nn:        types.NamespacedName{Namespace: "default", Name: "ingress-annotator"},
 			requestNN: types.NamespacedName{Namespace: "default", Name: "ingress-annotator"},
 			want:      ctrl.Result{},
 		},
 		{
-			name:      "Valid ConfigMap should process without errors or requeue",
+			name:      "Process valid ConfigMap without errors or requeue",
 			cm:        createConfigMap("default", "ingress-annotator", "rule1:\n  key1: value1"),
 			nn:        types.NamespacedName{Namespace: "default", Name: "ingress-annotator"},
 			requestNN: types.NamespacedName{Namespace: "default", Name: "ingress-annotator"},
 			want:      ctrl.Result{},
 		},
 		{
-			name:      "Valid ConfigMap but different request name should not return errors",
+			name:      "No errors when request name differs from ConfigMap name",
 			cm:        createConfigMap("default", "ingress-annotator", "rule1:\n  key1: value1"),
 			nn:        types.NamespacedName{Namespace: "default", Name: "ingress-annotator"},
 			requestNN: types.NamespacedName{Namespace: "default", Name: "xxx"},
@@ -144,8 +158,48 @@ func TestConfigMapReconciler_Reconcile(t *testing.T) {
 			} else {
 				assert.EqualError(t, err, tc.wantError)
 			}
-
 			assert.Equal(t, tc.want, got)
 		})
 	}
 }
+
+func TestConfigMapReconciler_annotateAllIngresses(t *testing.T) {
+	ingress1 := &networkingv1.Ingress{ObjectMeta: metav1.ObjectMeta{Name: "ingress1", Namespace: "default"}}
+	ingress2 := &networkingv1.Ingress{ObjectMeta: metav1.ObjectMeta{Name: "ingress2", Namespace: "default"}}
+
+	testCases := []struct {
+		clientOpts *fakeclient.ClientOpts
+		wantError  string
+	}{
+		{
+			clientOpts: nil,
+			wantError:  "",
+		},
+		{
+			clientOpts: &fakeclient.ClientOpts{GetError: true},
+			wantError:  "failed to annotateIngress: failed to get ingress default/ingress1: mocked GetError",
+		},
+		{
+			clientOpts: &fakeclient.ClientOpts{UpdateError: true},
+			wantError:  "failed to annotateIngress: failed to update ingress default/ingress1: mocked UpdateError",
+		},
+		{
+			clientOpts: &fakeclient.ClientOpts{UpdateConflictError: true},
+			wantError:  "failed to annotateIngress: mocked UpdateConflictError: Operation cannot be fulfilled on ingresses.networking.k8s.io \"ingress1\": the object has been modified; please apply your changes to the latest version and try again",
+		},
+	}
+	for i, tc := range testCases {
+		t.Run(testcase.Name(i), func(t *testing.T) {
+			client := fakeclient.NewClient(tc.clientOpts, ingress1, ingress2)
+			reconciler := &ConfigMapReconciler{
+				Client: client,
+			}
+			err := reconciler.annotateAllIngresses(context.TODO())
+			if tc.wantError == "" {
+				assert.NoError(t, err)
+			} else {
+				assert.EqualError(t, err, tc.wantError)
+			}
+		})
+	}
+}

controllers/ingresscontroller/ingress_controller.go

@@ -146,7 +146,7 @@ func (r *IngressReconciler) addNewAnnotations(scope *ingressScope) {
 	}
 
 	b := util.MustMarshalJSON(newAnnotations)
-	scope.updatedAnnotations[model.ManagedAnnotationsKey] = string(b)
+	scope.updatedAnnotations[model.ManagedAnnotationsKey] = string(b) + "\n"
 }
 
 func (r *IngressReconciler) getNewAnnotations(scope *ingressScope) model.Annotations {

controllers/ingresscontroller/ingress_controller_test.go

@@ -92,7 +92,7 @@ func TestIngressReconciler_Reconcile(t *testing.T) {
 			},
 			wantResult: ctrl.Result{},
 			wantAnnotations: map[string]string{
-				"annotator.ingress.kubernetes.io/managed-annotations": `{"new-key":"new-value"}`,
+				"annotator.ingress.kubernetes.io/managed-annotations": "{\"new-key\":\"new-value\"}\n",
 				"annotator.ingress.kubernetes.io/rules":               "rule1",
 				"new-key":                                             "new-value",
 			},
@@ -105,7 +105,7 @@ func TestIngressReconciler_Reconcile(t *testing.T) {
 			},
 			wantResult: ctrl.Result{},
 			wantAnnotations: map[string]string{
-				"annotator.ingress.kubernetes.io/managed-annotations": `{"new-key":"new-value"}`,
+				"annotator.ingress.kubernetes.io/managed-annotations": "{\"new-key\":\"new-value\"}\n",
 				"annotator.ingress.kubernetes.io/rules":               "rule1",
 				"new-key":                                             "new-value",
 			},
@@ -114,13 +114,13 @@ func TestIngressReconciler_Reconcile(t *testing.T) {
 			name:      "ValidIngressWithPreExistingAnnotations_ShouldRetainExistingAnnotations",
 			namespace: &corev1.Namespace{ObjectMeta: ctrl.ObjectMeta{Name: "default"}},
 			ingressAnnotations: map[string]string{
-				"annotator.ingress.kubernetes.io/managed-annotations": `{"new-key":"new-value"}`,
+				"annotator.ingress.kubernetes.io/managed-annotations": "{\"new-key\":\"new-value\"}\n",
 				"annotator.ingress.kubernetes.io/rules":               "rule1",
 				"new-key":                                             "new-value",
 			},
 			wantResult: ctrl.Result{},
 			wantAnnotations: map[string]string{
-				"annotator.ingress.kubernetes.io/managed-annotations": `{"new-key":"new-value"}`,
+				"annotator.ingress.kubernetes.io/managed-annotations": "{\"new-key\":\"new-value\"}\n",
 				"annotator.ingress.kubernetes.io/rules":               "rule1",
 				"new-key":                                             "new-value",
 			},
@@ -140,13 +140,13 @@ func TestIngressReconciler_Reconcile(t *testing.T) {
 			name:      "NoChangesDetected_ShouldReturnEarlyWithoutUpdates",
 			namespace: &corev1.Namespace{ObjectMeta: ctrl.ObjectMeta{Name: "default"}},
 			ingressAnnotations: map[string]string{
-				"annotator.ingress.kubernetes.io/managed-annotations": `{"new-key":"new-value"}`,
+				"annotator.ingress.kubernetes.io/managed-annotations": "{\"new-key\":\"new-value\"}\n",
 				"annotator.ingress.kubernetes.io/rules":               "rule1",
 				"new-key":                                             "old-value",
 			},
 			wantResult: ctrl.Result{},
 			wantAnnotations: map[string]string{
-				"annotator.ingress.kubernetes.io/managed-annotations": `{"new-key":"new-value"}`,
+				"annotator.ingress.kubernetes.io/managed-annotations": "{\"new-key\":\"new-value\"}\n",
 				"annotator.ingress.kubernetes.io/rules":               "rule1",
 				"new-key":                                             "new-value",
 			},
@@ -173,7 +173,7 @@ func TestIngressReconciler_Reconcile(t *testing.T) {
 			clientOpts: &fakeclient.ClientOpts{UpdateError: true},
 			namespace:  &corev1.Namespace{ObjectMeta: ctrl.ObjectMeta{Name: "default"}},
 			ingressAnnotations: map[string]string{
-				"annotator.ingress.kubernetes.io/managed-annotations": `{"new-key":"new-value"}`,
+				"annotator.ingress.kubernetes.io/managed-annotations": "{\"new-key\":\"new-value\"}\n",
 				"annotator.ingress.kubernetes.io/rules":               "rule1",
 			},
 			wantResult: ctrl.Result{RequeueAfter: 30 * time.Second},

controllers/namespacecontroller/namespace_controller.go

@@ -65,7 +65,7 @@ func (r *NamespaceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 	logger.Info("Reconciling Namespace")
 
 	if err := r.annotateIngressesInNamespace(ctx, req.Namespace); err != nil {
-		return ctrl.Result{}, fmt.Errorf("failed to annotate ingresses: %w", err)
+		return ctrl.Result{}, fmt.Errorf("failed to annotateIngressesInNamespace: %w", err)
 	}
 
 	logger.Info("Reconciled Namespace successfully")
@@ -81,7 +81,7 @@ func (r *NamespaceReconciler) annotateIngressesInNamespace(ctx context.Context,
 
 	for _, ing := range ingressList.Items {
 		if err := r.annotateIngress(ctx, ing); err != nil {
-			return fmt.Errorf("failed to annotate ingress: %w", err)
+			return fmt.Errorf("failed to annotateIngress: %w", err)
 		}
 	}
 
@@ -91,7 +91,7 @@ func (r *NamespaceReconciler) annotateIngressesInNamespace(ctx context.Context,
 func (r *NamespaceReconciler) annotateIngress(ctx context.Context, ing networkingv1.Ingress) error {
 	return retry.RetryOnConflict(retry.DefaultRetry, func() error {
 		if err := r.Get(ctx, client.ObjectKey{Name: ing.Name, Namespace: ing.Namespace}, &ing); err != nil {
-			return fmt.Errorf("failed to get latest ingress %s/%s: %w", ing.Namespace, ing.Name, err)
+			return fmt.Errorf("failed to get ingress %s/%s: %w", ing.Namespace, ing.Name, err)
 		}
 		ing.SetAnnotations(map[string]string{model.ReconcileKey: "true"})
 		if err := r.Update(ctx, &ing); err != nil {

controllers/namespacecontroller/namespace_controller_test.go

@@ -62,13 +62,13 @@ func TestNamespaceReconciler_Reconcile(t *testing.T) {
 			namespace:  &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-namespace"}},
 			clientOpts: &fakeclient.ClientOpts{ListError: true},
 			wantResult: ctrl.Result{},
-			wantError:  "failed to annotate ingresses: failed to list ingresses: mocked ListError",
+			wantError:  "failed to annotateIngressesInNamespace: failed to list ingresses: mocked ListError",
 		},
 		{
 			namespace:  &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-namespace"}},
 			clientOpts: &fakeclient.ClientOpts{UpdateError: true},
 			wantResult: ctrl.Result{},
-			wantError:  "failed to annotate ingresses: failed to annotate ingress: failed to update ingress test-namespace/test-ingress: mocked UpdateError",
+			wantError:  "failed to annotateIngressesInNamespace: failed to annotateIngress: failed to update ingress test-namespace/test-ingress: mocked UpdateError",
 		},
 	}
 
@@ -117,7 +117,7 @@ func TestNamespaceReconciler_annotateIngress(t *testing.T) {
 			name:       "get error",
 			ingress:    &networkingv1.Ingress{ObjectMeta: metav1.ObjectMeta{Name: "test-ingress", Namespace: "test-namespace"}},
 			clientOpts: &fakeclient.ClientOpts{GetError: true},
-			wantError:  "failed to get latest ingress test-namespace/test-ingress: mocked GetError",
+			wantError:  "failed to get ingress test-namespace/test-ingress: mocked GetError",
 		},
 		{
 			name:       "update error",
@@ -133,8 +133,8 @@ func TestNamespaceReconciler_annotateIngress(t *testing.T) {
 		},
 	}
 
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
+	for i, tt := range tests {
+		t.Run(testcase.Name(i, tt.name), func(t *testing.T) {
 			client := fakeclient.NewClient(tt.clientOpts, tt.ingress)
 			r := &NamespaceReconciler{
 				Client: client,