diff --git a/internal/framework/controller/predicate/annotation.go b/internal/framework/controller/predicate/annotation.go index 46b48660de..c6c34585f6 100644 --- a/internal/framework/controller/predicate/annotation.go +++ b/internal/framework/controller/predicate/annotation.go @@ -19,24 +19,24 @@ type AnnotationPredicate struct { } // Create filters CreateEvents based on the Annotation. -func (cp AnnotationPredicate) Create(e event.CreateEvent) bool { +func (ap AnnotationPredicate) Create(e event.CreateEvent) bool { if e.Object == nil { return false } - _, ok := e.Object.GetAnnotations()[cp.Annotation] + _, ok := e.Object.GetAnnotations()[ap.Annotation] return ok } // Update filters UpdateEvents based on the Annotation. -func (cp AnnotationPredicate) Update(e event.UpdateEvent) bool { +func (ap AnnotationPredicate) Update(e event.UpdateEvent) bool { if e.ObjectOld == nil || e.ObjectNew == nil { // this case should not happen return false } - oldAnnotationVal := e.ObjectOld.GetAnnotations()[cp.Annotation] - newAnnotationVal := e.ObjectNew.GetAnnotations()[cp.Annotation] + oldAnnotationVal := e.ObjectOld.GetAnnotations()[ap.Annotation] + newAnnotationVal := e.ObjectNew.GetAnnotations()[ap.Annotation] return oldAnnotationVal != newAnnotationVal } @@ -52,7 +52,7 @@ type RestartDeploymentAnnotationPredicate struct { } // Update filters UpdateEvents based on if the annotation is present or changed. -func (cp RestartDeploymentAnnotationPredicate) Update(e event.UpdateEvent) bool { +func (RestartDeploymentAnnotationPredicate) Update(e event.UpdateEvent) bool { if e.ObjectOld == nil || e.ObjectNew == nil { // this case should not happen return false diff --git a/internal/framework/controller/predicate/secret.go b/internal/framework/controller/predicate/secret.go new file mode 100644 index 0000000000..0e28679d89 --- /dev/null +++ b/internal/framework/controller/predicate/secret.go @@ -0,0 +1,77 @@ +package predicate + +import ( + "slices" + + corev1 "k8s.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/predicate" +) + +// SecretNamePredicate implements a predicate function that returns true if the Secret matches the expected +// namespace and one of the expected names. +type SecretNamePredicate struct { + predicate.Funcs + Namespace string + SecretNames []string +} + +// Create filters CreateEvents based on the Secret name. +func (sp SecretNamePredicate) Create(e event.CreateEvent) bool { + if e.Object == nil { + return false + } + + if secret, ok := e.Object.(*corev1.Secret); ok { + return secretMatches(secret, sp.Namespace, sp.SecretNames) + } + + return false +} + +// Update filters UpdateEvents based on the Secret name. +func (sp SecretNamePredicate) Update(e event.UpdateEvent) bool { + if e.ObjectNew == nil { + return false + } + + if secret, ok := e.ObjectNew.(*corev1.Secret); ok { + return secretMatches(secret, sp.Namespace, sp.SecretNames) + } + + return false +} + +// Delete filters DeleteEvents based on the Secret name. +func (sp SecretNamePredicate) Delete(e event.DeleteEvent) bool { + if e.Object == nil { + return false + } + + if secret, ok := e.Object.(*corev1.Secret); ok { + return secretMatches(secret, sp.Namespace, sp.SecretNames) + } + + return false +} + +// Generic filters GenericEvents based on the Secret name. +func (sp SecretNamePredicate) Generic(e event.GenericEvent) bool { + if e.Object == nil { + return false + } + + if secret, ok := e.Object.(*corev1.Secret); ok { + return secretMatches(secret, sp.Namespace, sp.SecretNames) + } + + return false +} + +func secretMatches(secret *corev1.Secret, namespace string, names []string) bool { + if secret.GetNamespace() != namespace { + return false + } + + return slices.Contains(names, secret.GetName()) +} diff --git a/internal/framework/controller/predicate/secret_test.go b/internal/framework/controller/predicate/secret_test.go new file mode 100644 index 0000000000..a9574dbb8c --- /dev/null +++ b/internal/framework/controller/predicate/secret_test.go @@ -0,0 +1,194 @@ +package predicate + +import ( + "testing" + + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/event" +) + +func TestSecretNamePredicate(t *testing.T) { + t.Parallel() + + pred := SecretNamePredicate{ + Namespace: "test-namespace", + SecretNames: []string{"secret1", "secret2"}, + } + + tests := []struct { + createEvent *event.CreateEvent + updateEvent *event.UpdateEvent + deleteEvent *event.DeleteEvent + genericEvent *event.GenericEvent + name string + expUpdate bool + }{ + { + name: "Create event with matching secret", + createEvent: &event.CreateEvent{ + Object: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "secret1", + Namespace: "test-namespace", + }, + }, + }, + expUpdate: true, + }, + { + name: "Create event with non-matching secret", + createEvent: &event.CreateEvent{ + Object: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "secret3", + Namespace: "test-namespace", + }, + }, + }, + expUpdate: false, + }, + { + name: "Create event with non-matching namespace", + createEvent: &event.CreateEvent{ + Object: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "secret1", + Namespace: "other-namespace", + }, + }, + }, + expUpdate: false, + }, + { + name: "Update event with matching secret", + updateEvent: &event.UpdateEvent{ + ObjectNew: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "secret2", + Namespace: "test-namespace", + }, + }, + }, + expUpdate: true, + }, + { + name: "Update event with non-matching secret", + updateEvent: &event.UpdateEvent{ + ObjectNew: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "secret3", + Namespace: "test-namespace", + }, + }, + }, + expUpdate: false, + }, + { + name: "Update event with non-matching namespace", + updateEvent: &event.UpdateEvent{ + ObjectNew: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "secret1", + Namespace: "other-namespace", + }, + }, + }, + expUpdate: false, + }, + { + name: "Delete event with matching secret", + deleteEvent: &event.DeleteEvent{ + Object: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "secret1", + Namespace: "test-namespace", + }, + }, + }, + expUpdate: true, + }, + { + name: "Delete event with non-matching secret", + deleteEvent: &event.DeleteEvent{ + Object: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "secret3", + Namespace: "test-namespace", + }, + }, + }, + expUpdate: false, + }, + { + name: "Delete event with non-matching namespace", + deleteEvent: &event.DeleteEvent{ + Object: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "secret1", + Namespace: "other-namespace", + }, + }, + }, + expUpdate: false, + }, + { + name: "Generic event with matching secret", + genericEvent: &event.GenericEvent{ + Object: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "secret1", + Namespace: "test-namespace", + }, + }, + }, + expUpdate: true, + }, + { + name: "Generic event with non-matching secret", + genericEvent: &event.GenericEvent{ + Object: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "secret3", + Namespace: "test-namespace", + }, + }, + }, + expUpdate: false, + }, + { + name: "Generic event with non-matching namespace", + genericEvent: &event.GenericEvent{ + Object: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "secret1", + Namespace: "other-namespace", + }, + }, + }, + expUpdate: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + var result bool + switch { + case test.createEvent != nil: + result = pred.Create(*test.createEvent) + case test.updateEvent != nil: + result = pred.Update(*test.updateEvent) + case test.deleteEvent != nil: + result = pred.Delete(*test.deleteEvent) + default: + result = pred.Generic(*test.genericEvent) + } + + g.Expect(test.expUpdate).To(Equal(result)) + }) + } +} diff --git a/internal/framework/controller/resource.go b/internal/framework/controller/resource.go index 2fff439a50..464d2ee90f 100644 --- a/internal/framework/controller/resource.go +++ b/internal/framework/controller/resource.go @@ -1,9 +1,22 @@ package controller -import "fmt" +import ( + "fmt" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" +) // CreateNginxResourceName creates the base resource name for all nginx resources // created by the control plane. func CreateNginxResourceName(prefix, suffix string) string { return fmt.Sprintf("%s-%s", prefix, suffix) } + +// ObjectMetaToNamespacedName converts ObjectMeta to NamespacedName. +func ObjectMetaToNamespacedName(meta metav1.ObjectMeta) types.NamespacedName { + return types.NamespacedName{ + Namespace: meta.Namespace, + Name: meta.Name, + } +} diff --git a/internal/mode/static/provisioner/eventloop.go b/internal/mode/static/provisioner/eventloop.go index 5f080156bd..6b50e79e38 100644 --- a/internal/mode/static/provisioner/eventloop.go +++ b/internal/mode/static/provisioner/eventloop.go @@ -18,6 +18,7 @@ import ( "github.com/nginx/nginx-gateway-fabric/internal/framework/controller/predicate" "github.com/nginx/nginx-gateway-fabric/internal/framework/events" ngftypes "github.com/nginx/nginx-gateway-fabric/internal/framework/types" + "github.com/nginx/nginx-gateway-fabric/internal/mode/static/config" ) func newEventLoop( @@ -26,11 +27,30 @@ func newEventLoop( handler *eventHandler, logger logr.Logger, selector metav1.LabelSelector, + ngfNamespace string, + dockerSecrets []string, + usageConfig *config.UsageReportConfig, ) (*events.EventLoop, error) { nginxResourceLabelPredicate := predicate.NginxLabelPredicate(selector) + secretsToWatch := make([]string, 0, len(dockerSecrets)+3) + secretsToWatch = append(secretsToWatch, dockerSecrets...) + + if usageConfig != nil { + if usageConfig.SecretName != "" { + secretsToWatch = append(secretsToWatch, usageConfig.SecretName) + } + if usageConfig.CASecretName != "" { + secretsToWatch = append(secretsToWatch, usageConfig.CASecretName) + } + if usageConfig.ClientSSLSecretName != "" { + secretsToWatch = append(secretsToWatch, usageConfig.ClientSSLSecretName) + } + } + controllerRegCfgs := []struct { objectType ngftypes.ObjectType + name string options []controller.Option }{ { @@ -85,15 +105,18 @@ func newEventLoop( options: []controller.Option{ controller.WithK8sPredicate( k8spredicate.And( - k8spredicate.GenerationChangedPredicate{}, - nginxResourceLabelPredicate, + k8spredicate.ResourceVersionChangedPredicate{}, + k8spredicate.Or( + nginxResourceLabelPredicate, + predicate.SecretNamePredicate{Namespace: ngfNamespace, SecretNames: secretsToWatch}, + ), ), ), }, }, } - eventCh := make(chan interface{}) + eventCh := make(chan any) for _, regCfg := range controllerRegCfgs { gvk, err := apiutil.GVKForObject(regCfg.objectType, mgr.GetScheme()) if err != nil { diff --git a/internal/mode/static/provisioner/handler.go b/internal/mode/static/provisioner/handler.go index 5885373213..b3d7fe5efd 100644 --- a/internal/mode/static/provisioner/handler.go +++ b/internal/mode/static/provisioner/handler.go @@ -2,7 +2,9 @@ package provisioner import ( "context" + "errors" "fmt" + "strings" "github.com/go-logr/logr" appsv1 "k8s.io/api/apps/v1" @@ -49,6 +51,7 @@ func newEventHandler( }, nil } +//nolint:gocyclo // will refactor at some point func (h *eventHandler) HandleEventBatch(ctx context.Context, logger logr.Logger, batch events.EventBatch) { for _, event := range batch { switch e := event.(type) { @@ -56,7 +59,7 @@ func (h *eventHandler) HandleEventBatch(ctx context.Context, logger logr.Logger, switch obj := e.Resource.(type) { case *gatewayv1.Gateway: h.store.updateGateway(obj) - case *appsv1.Deployment, *corev1.ServiceAccount, *corev1.ConfigMap, *corev1.Secret: + case *appsv1.Deployment, *corev1.ServiceAccount, *corev1.ConfigMap: objLabels := labels.Set(obj.GetLabels()) if h.labelSelector.Matches(objLabels) { gatewayName := objLabels.Get(controller.GatewayLabel) @@ -83,6 +86,20 @@ func (h *eventHandler) HandleEventBatch(ctx context.Context, logger logr.Logger, } h.provisioner.cfg.StatusQueue.Enqueue(statusUpdate) } + case *corev1.Secret: + objLabels := labels.Set(obj.GetLabels()) + if h.labelSelector.Matches(objLabels) { + gatewayName := objLabels.Get(controller.GatewayLabel) + gatewayNSName := types.NamespacedName{Namespace: obj.GetNamespace(), Name: gatewayName} + + if err := h.updateOrDeleteResources(ctx, obj, gatewayNSName); err != nil { + logger.Error(err, "error handling resource update") + } + } else if h.provisioner.isUserSecret(obj.GetName()) { + if err := h.provisionResourcesForAllGateways(ctx); err != nil { + logger.Error(err, "error updating resources") + } + } default: panic(fmt.Errorf("unknown resource type %T", e.Resource)) } @@ -93,10 +110,20 @@ func (h *eventHandler) HandleEventBatch(ctx context.Context, logger logr.Logger, logger.Error(err, "error deprovisioning nginx resources") } h.store.deleteGateway(e.NamespacedName) - case *appsv1.Deployment, *corev1.Service, *corev1.ServiceAccount, *corev1.ConfigMap, *corev1.Secret: + case *appsv1.Deployment, *corev1.Service, *corev1.ServiceAccount, *corev1.ConfigMap: if err := h.reprovisionResources(ctx, e); err != nil { logger.Error(err, "error re-provisioning nginx resources") } + case *corev1.Secret: + if h.provisioner.isUserSecret(e.NamespacedName.Name) { + if err := h.deprovisionSecretsForAllGateways(ctx, e.NamespacedName.Name); err != nil { + logger.Error(err, "error removing secrets") + } + } else { + if err := h.reprovisionResources(ctx, e); err != nil { + logger.Error(err, "error re-provisioning nginx resources") + } + } default: panic(fmt.Errorf("unknown resource type %T", e.Type)) } @@ -128,7 +155,17 @@ func (h *eventHandler) updateOrDeleteResources( } h.store.registerResourceInGatewayConfig(gatewayNSName, obj) + if err := h.provisionResources(ctx, gatewayNSName); err != nil { + return fmt.Errorf("error updating nginx resource: %w", err) + } + + return nil +} +func (h *eventHandler) provisionResources( + ctx context.Context, + gatewayNSName types.NamespacedName, +) error { resources := h.store.getNginxResourcesForGateway(gatewayNSName) if resources.Gateway != nil { resourceName := controller.CreateNginxResourceName(gatewayNSName.Name, h.gcName) @@ -160,3 +197,68 @@ func (h *eventHandler) reprovisionResources(ctx context.Context, event *events.D } return nil } + +// provisionResourcesForAllGateways is called when a resource is updated that needs to be applied +// to all Gateway deployments. For example, NGINX Plus secrets. +func (h *eventHandler) provisionResourcesForAllGateways(ctx context.Context) error { + var allErrs []error + gateways := h.store.getGateways() + for gateway := range gateways { + if err := h.provisionResources(ctx, gateway); err != nil { + allErrs = append(allErrs, err) + } + } + + return errors.Join(allErrs...) +} + +// deprovisionSecretsForAllGateways cleans up any secrets that a user deleted that were duplicated +// for all Gateways. For example, NGINX Plus secrets. +func (h *eventHandler) deprovisionSecretsForAllGateways(ctx context.Context, secret string) error { + var allErrs []error + + gateways := h.store.getGateways() + for gateway := range gateways { + resources := h.store.getNginxResourcesForGateway(gateway) + if resources == nil { + continue + } + + switch { + case strings.HasSuffix(resources.PlusJWTSecret.Name, secret): + if err := h.provisioner.deleteSecret( + ctx, + controller.ObjectMetaToNamespacedName(resources.PlusJWTSecret), + ); err != nil { + allErrs = append(allErrs, err) + } + case strings.HasSuffix(resources.PlusCASecret.Name, secret): + if err := h.provisioner.deleteSecret( + ctx, + controller.ObjectMetaToNamespacedName(resources.PlusCASecret), + ); err != nil { + allErrs = append(allErrs, err) + } + case strings.HasSuffix(resources.PlusClientSSLSecret.Name, secret): + if err := h.provisioner.deleteSecret( + ctx, + controller.ObjectMetaToNamespacedName(resources.PlusClientSSLSecret), + ); err != nil { + allErrs = append(allErrs, err) + } + default: + for _, dockerSecret := range resources.DockerSecrets { + if strings.HasSuffix(dockerSecret.Name, secret) { + if err := h.provisioner.deleteSecret( + ctx, + controller.ObjectMetaToNamespacedName(dockerSecret), + ); err != nil { + allErrs = append(allErrs, err) + } + } + } + } + } + + return errors.Join(allErrs...) +} diff --git a/internal/mode/static/provisioner/handler_test.go b/internal/mode/static/provisioner/handler_test.go index bc3aa61d08..720690b972 100644 --- a/internal/mode/static/provisioner/handler_test.go +++ b/internal/mode/static/provisioner/handler_test.go @@ -9,6 +9,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" @@ -22,11 +23,9 @@ func TestHandleEventBatch_Upsert(t *testing.T) { t.Parallel() g := NewWithT(t) - store := newStore(nil, "", "", "") + store := newStore([]string{dockerTestSecretName}, jwtTestSecretName, "", "") provisioner, fakeClient, _ := defaultNginxProvisioner() provisioner.cfg.StatusQueue = status.NewQueue() - provisioner.cfg.Plus = false - provisioner.cfg.NginxDockerSecretNames = nil labelSelector := metav1.LabelSelector{ MatchLabels: map[string]string{"app": "nginx"}, @@ -57,15 +56,57 @@ func TestHandleEventBatch_Upsert(t *testing.T) { service := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-service", + Name: "gw-nginx", + Namespace: "default", + Labels: map[string]string{"app": "nginx", controller.GatewayLabel: "test-gateway"}, + }, + } + + jwtSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gw-nginx-" + jwtTestSecretName, Namespace: "default", Labels: map[string]string{"app": "nginx", controller.GatewayLabel: "test-gateway"}, }, + Data: map[string][]byte{ + "data": []byte("oldData"), + }, + } + + userJwtSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: jwtTestSecretName, + Namespace: ngfNamespace, + }, + Data: map[string][]byte{ + "data": []byte("oldData"), + }, + } + g.Expect(fakeClient.Create(ctx, userJwtSecret)).To(Succeed()) + + dockerSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gw-nginx-" + dockerTestSecretName, + Namespace: "default", + }, + Data: map[string][]byte{ + "data": []byte("oldDockerData"), + }, } + userDockerSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: dockerTestSecretName, + Namespace: ngfNamespace, + }, + Data: map[string][]byte{ + "data": []byte("oldDockerData"), + }, + } + g.Expect(fakeClient.Create(ctx, userDockerSecret)).To(Succeed()) + // Test handling Gateway upsertEvent := &events.UpsertEvent{Resource: gateway} - batch := events.EventBatch{upsertEvent} handler.HandleEventBatch(ctx, logger, batch) @@ -90,6 +131,44 @@ func TestHandleEventBatch_Upsert(t *testing.T) { g.Expect(provisioner.cfg.StatusQueue.Dequeue(ctx)).ToNot(BeNil()) + // Test handling provisioned Secret + upsertEvent = &events.UpsertEvent{Resource: jwtSecret} + batch = events.EventBatch{upsertEvent} + handler.HandleEventBatch(ctx, logger, batch) + + g.Expect(fakeClient.Get(ctx, client.ObjectKeyFromObject(jwtSecret), &corev1.Secret{})).To(Succeed()) + + // Test handling user Plus Secret + secret := &corev1.Secret{} + g.Expect(fakeClient.Get(ctx, client.ObjectKeyFromObject(jwtSecret), secret)).To(Succeed()) + g.Expect(secret.Data).To(HaveKey("data")) + g.Expect(secret.Data["data"]).To(Equal([]byte("oldData"))) + + userJwtSecret.Data["data"] = []byte("newData") + g.Expect(fakeClient.Update(ctx, userJwtSecret)).To(Succeed()) + upsertEvent = &events.UpsertEvent{Resource: userJwtSecret} + batch = events.EventBatch{upsertEvent} + handler.HandleEventBatch(ctx, logger, batch) + + g.Expect(fakeClient.Get(ctx, client.ObjectKeyFromObject(jwtSecret), secret)).To(Succeed()) + g.Expect(secret.Data).To(HaveKey("data")) + g.Expect(secret.Data["data"]).To(Equal([]byte("newData"))) + + // Test handling user Docker Secret + g.Expect(fakeClient.Get(ctx, client.ObjectKeyFromObject(dockerSecret), secret)).To(Succeed()) + g.Expect(secret.Data).To(HaveKey("data")) + g.Expect(secret.Data["data"]).To(Equal([]byte("oldDockerData"))) + + userDockerSecret.Data["data"] = []byte("newDockerData") + g.Expect(fakeClient.Update(ctx, userDockerSecret)).To(Succeed()) + upsertEvent = &events.UpsertEvent{Resource: userDockerSecret} + batch = events.EventBatch{upsertEvent} + handler.HandleEventBatch(ctx, logger, batch) + + g.Expect(fakeClient.Get(ctx, client.ObjectKeyFromObject(dockerSecret), secret)).To(Succeed()) + g.Expect(secret.Data).To(HaveKey("data")) + g.Expect(secret.Data["data"]).To(Equal([]byte("newDockerData"))) + // remove Gateway from store and verify that Deployment UpsertEvent results in deletion of resource store.deleteGateway(client.ObjectKeyFromObject(gateway)) g.Expect(store.getGateway(client.ObjectKeyFromObject(gateway))).To(BeNil()) @@ -117,11 +196,9 @@ func TestHandleEventBatch_Delete(t *testing.T) { t.Parallel() g := NewWithT(t) - store := newStore(nil, "", "", "") + store := newStore([]string{dockerTestSecretName}, jwtTestSecretName, caTestSecretName, clientTestSecretName) provisioner, fakeClient, _ := defaultNginxProvisioner() provisioner.cfg.StatusQueue = status.NewQueue() - provisioner.cfg.Plus = false - provisioner.cfg.NginxDockerSecretNames = nil labelSelector := metav1.LabelSelector{ MatchLabels: map[string]string{"app": "nginx"}, @@ -134,6 +211,7 @@ func TestHandleEventBatch_Delete(t *testing.T) { ctx := context.TODO() logger := logr.Discard() + // initialize resources gateway := &gatewayv1.Gateway{ ObjectMeta: metav1.ObjectMeta{ Name: "gw", @@ -155,15 +233,89 @@ func TestHandleEventBatch_Delete(t *testing.T) { }, } + jwtSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gw-nginx-" + jwtTestSecretName, + Namespace: "default", + Labels: map[string]string{"app": "nginx", controller.GatewayLabel: "gw"}, + }, + } + + userJwtSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: jwtTestSecretName, + Namespace: ngfNamespace, + }, + } + g.Expect(fakeClient.Create(ctx, userJwtSecret)).To(Succeed()) + + userCASecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: caTestSecretName, + Namespace: ngfNamespace, + }, + } + g.Expect(fakeClient.Create(ctx, userCASecret)).To(Succeed()) + + userClientSSLSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: clientTestSecretName, + Namespace: ngfNamespace, + }, + } + g.Expect(fakeClient.Create(ctx, userClientSSLSecret)).To(Succeed()) + + userDockerSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: dockerTestSecretName, + Namespace: ngfNamespace, + }, + } + g.Expect(fakeClient.Create(ctx, userDockerSecret)).To(Succeed()) + + upsertEvent := &events.UpsertEvent{Resource: gateway} + batch := events.EventBatch{upsertEvent} + handler.HandleEventBatch(ctx, logger, batch) store.registerResourceInGatewayConfig(client.ObjectKeyFromObject(gateway), deployment) // if deployment is deleted, it should be re-created since Gateway still exists deleteEvent := &events.DeleteEvent{Type: deployment, NamespacedName: client.ObjectKeyFromObject(deployment)} - batch := events.EventBatch{deleteEvent} + batch = events.EventBatch{deleteEvent} handler.HandleEventBatch(ctx, logger, batch) g.Expect(fakeClient.Get(ctx, client.ObjectKeyFromObject(deployment), &appsv1.Deployment{})).To(Succeed()) + // if provisioned secret is deleted, it should be re-created + deleteEvent = &events.DeleteEvent{Type: jwtSecret, NamespacedName: client.ObjectKeyFromObject(jwtSecret)} + batch = events.EventBatch{deleteEvent} + handler.HandleEventBatch(ctx, logger, batch) + + g.Expect(fakeClient.Get(ctx, client.ObjectKeyFromObject(jwtSecret), &corev1.Secret{})).To(Succeed()) + + // if user-provided secrets are deleted, then delete the duplicates of them + verifySecret := func(name string, userSecret *corev1.Secret) { + key := types.NamespacedName{ + Name: "gw-nginx-" + name, + Namespace: "default", + } + + secret := &corev1.Secret{} + g.Expect(fakeClient.Get(ctx, key, secret)).To(Succeed()) + store.registerResourceInGatewayConfig(client.ObjectKeyFromObject(gateway), secret) + + g.Expect(fakeClient.Delete(ctx, userSecret)).To(Succeed()) + deleteEvent = &events.DeleteEvent{Type: userSecret, NamespacedName: client.ObjectKeyFromObject(userSecret)} + batch = events.EventBatch{deleteEvent} + handler.HandleEventBatch(ctx, logger, batch) + + g.Expect(fakeClient.Get(ctx, key, &corev1.Secret{})).ToNot(Succeed()) + } + + verifySecret(jwtTestSecretName, userJwtSecret) + verifySecret(caTestSecretName, userCASecret) + verifySecret(clientTestSecretName, userClientSSLSecret) + verifySecret(dockerTestSecretName, userDockerSecret) + // delete Gateway deleteEvent = &events.DeleteEvent{Type: gateway, NamespacedName: client.ObjectKeyFromObject(gateway)} batch = events.EventBatch{deleteEvent} diff --git a/internal/mode/static/provisioner/provisioner.go b/internal/mode/static/provisioner/provisioner.go index 7165805a44..71439f4b4e 100644 --- a/internal/mode/static/provisioner/provisioner.go +++ b/internal/mode/static/provisioner/provisioner.go @@ -3,6 +3,7 @@ package provisioner import ( "context" "fmt" + "slices" "strings" "sync" "time" @@ -105,7 +106,16 @@ func NewNginxProvisioner( return nil, nil, fmt.Errorf("error initializing eventHandler: %w", err) } - eventLoop, err := newEventLoop(ctx, mgr, handler, cfg.Logger, selector) + eventLoop, err := newEventLoop( + ctx, + mgr, + handler, + cfg.Logger, + selector, + cfg.GatewayPodConfig.Namespace, + cfg.NginxDockerSecretNames, + cfg.PlusUsageConfig, + ) if err != nil { return nil, nil, err } @@ -162,7 +172,7 @@ func (p *NginxProvisioner) provisionNginx( objects, err := p.buildNginxResourceObjects(resourceName, gateway, nProxyCfg) if err != nil { - return fmt.Errorf("error provisioning nginx resources :%w", err) + p.cfg.Logger.Error(err, "error provisioning some nginx resources") } p.cfg.Logger.Info( @@ -275,7 +285,7 @@ func (p *NginxProvisioner) reprovisionNginx( objects, err := p.buildNginxResourceObjects(resourceName, gateway, nProxyCfg) if err != nil { - return fmt.Errorf("error provisioning nginx resources :%w", err) + p.cfg.Logger.Error(err, "error provisioning some nginx resources") } p.cfg.Logger.Info( @@ -341,6 +351,41 @@ func (p *NginxProvisioner) deprovisionNginx(ctx context.Context, gatewayNSName t return nil } +// isUserSecret determines if the provided secret name is a special user secret, +// for example an NGINX docker registry secret or NGINX Plus secret. +func (p *NginxProvisioner) isUserSecret(name string) bool { + if slices.Contains(p.cfg.NginxDockerSecretNames, name) { + return true + } + + if p.cfg.PlusUsageConfig != nil { + return name == p.cfg.PlusUsageConfig.SecretName || + name == p.cfg.PlusUsageConfig.CASecretName || + name == p.cfg.PlusUsageConfig.ClientSSLSecretName + } + + return false +} + +func (p *NginxProvisioner) deleteSecret(ctx context.Context, secretNSName types.NamespacedName) error { + if !p.isLeader() { + return nil + } + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretNSName.Name, + Namespace: secretNSName.Namespace, + }, + } + + if err := p.k8sClient.Delete(ctx, secret); err != nil && !apierrors.IsNotFound(err) { + return err + } + + return nil +} + // RegisterGateway is called by the main event handler when a Gateway API resource event occurs // and the graph is built. The provisioner updates the Gateway config in the store and then: // - If it's a valid Gateway, create or update nginx resources associated with the Gateway, if necessary. diff --git a/internal/mode/static/provisioner/provisioner_test.go b/internal/mode/static/provisioner/provisioner_test.go index 2c611912d8..8ef7873386 100644 --- a/internal/mode/static/provisioner/provisioner_test.go +++ b/internal/mode/static/provisioner/provisioner_test.go @@ -144,7 +144,7 @@ func defaultNginxProvisioner( deploymentStore := &agentfakes.FakeDeploymentStorer{} return &NginxProvisioner{ - store: newStore([]string{"docker-secret"}, "jwt-secret", "ca-secret", "client-ssl-secret"), + store: newStore([]string{dockerTestSecretName}, jwtTestSecretName, caTestSecretName, clientTestSecretName), k8sClient: fakeClient, cfg: Config{ DeploymentStore: deploymentStore, diff --git a/internal/mode/static/provisioner/store.go b/internal/mode/static/provisioner/store.go index 5a57d5cc99..ac63beb907 100644 --- a/internal/mode/static/provisioner/store.go +++ b/internal/mode/static/provisioner/store.go @@ -88,6 +88,13 @@ func (s *store) getGateway(nsName types.NamespacedName) *gatewayv1.Gateway { return s.gateways[nsName] } +func (s *store) getGateways() map[types.NamespacedName]*gatewayv1.Gateway { + s.lock.RLock() + defer s.lock.RUnlock() + + return s.gateways +} + // registerResourceInGatewayConfig adds or updates the provided resource in the tracking map. // If the object being updated is the Gateway, check if anything that we care about changed. This ensures that // we don't attempt to update nginx resources when the main event handler triggers this call with an unrelated event diff --git a/internal/mode/static/provisioner/store_test.go b/internal/mode/static/provisioner/store_test.go index 0358341f03..079736fde0 100644 --- a/internal/mode/static/provisioner/store_test.go +++ b/internal/mode/static/provisioner/store_test.go @@ -64,6 +64,37 @@ func TestDeleteGateway(t *testing.T) { g.Expect(store.getGateway(nsName)).To(BeNil()) } +func TestGetGateways(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + store := newStore(nil, "", "", "") + gateway1 := &gatewayv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-gateway-1", + Namespace: "default", + }, + } + gateway2 := &gatewayv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-gateway-2", + Namespace: "default", + }, + } + nsName1 := client.ObjectKeyFromObject(gateway1) + nsName2 := client.ObjectKeyFromObject(gateway2) + + store.updateGateway(gateway1) + store.updateGateway(gateway2) + + gateways := store.getGateways() + + g.Expect(gateways).To(HaveKey(nsName1)) + g.Expect(gateways).To(HaveKey(nsName2)) + g.Expect(gateways[nsName1]).To(Equal(gateway1)) + g.Expect(gateways[nsName2]).To(Equal(gateway2)) +} + func TestRegisterResourceInGatewayConfig(t *testing.T) { t.Parallel() g := NewWithT(t)