diff --git a/cmd/agent-manager.go b/cmd/agent-manager.go index 1366afb8..5490c0ea 100644 --- a/cmd/agent-manager.go +++ b/cmd/agent-manager.go @@ -79,6 +79,8 @@ type agentManagerOpts struct { // agent(tunnel driver) flags region string rootCAs string + + defaultDomainReclaimPolicy string } func agentCmd() *cobra.Command { @@ -107,6 +109,8 @@ func agentCmd() *cobra.Command { c.Flags().BoolVar(&opts.disableGatewayReferenceGrants, "disable-reference-grants", false, "Opts-out of requiring ReferenceGrants for cross namespace references in Gateway API config") c.Flags().BoolVar(&opts.enableFeatureBindings, "enable-feature-bindings", false, "Enables the Endpoint Bindings controller") + c.Flags().StringVar(&opts.defaultDomainReclaimPolicy, "default-domain-reclaim-policy", string(ingressv1alpha1.DomainReclaimPolicyDelete), "The default domain reclaim policy to apply to created domains") + opts.zapOpts = &zap.Options{} goFlagSet := flag.NewFlagSet("manager", flag.ContinueOnError) opts.zapOpts.BindFlags(goFlagSet) @@ -118,6 +122,11 @@ func agentCmd() *cobra.Command { func runAgentController(ctx context.Context, opts agentManagerOpts) error { ctrl.SetLogger(zap.New(zap.UseFlagOptions(opts.zapOpts))) + defaultDomainReclaimPolicy, err := validateDomainReclaimPolicy(opts.defaultDomainReclaimPolicy) + if err != nil { + return err + } + buildInfo := version.Get() setupLog.Info("starting agent-manager", "version", buildInfo.Version, "commit", buildInfo.GitCommit) @@ -180,11 +189,12 @@ func runAgentController(ctx context.Context, opts agentManagerOpts) error { } if err = (&agentcontroller.AgentEndpointReconciler{ - Client: mgr.GetClient(), - Log: ctrl.Log.WithName("controllers").WithName("agentendpoint"), - Scheme: mgr.GetScheme(), - Recorder: mgr.GetEventRecorderFor("agentendpoint-controller"), - TunnelDriver: td, + Client: mgr.GetClient(), + Log: ctrl.Log.WithName("controllers").WithName("agentendpoint"), + Scheme: mgr.GetScheme(), + Recorder: mgr.GetEventRecorderFor("agentendpoint-controller"), + TunnelDriver: td, + DefaultDomainReclaimPolicy: defaultDomainReclaimPolicy, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "AgentEndpoint") os.Exit(1) diff --git a/cmd/api-manager.go b/cmd/api-manager.go index ff177e6c..ddd6e5a2 100644 --- a/cmd/api-manager.go +++ b/cmd/api-manager.go @@ -130,6 +130,8 @@ type apiManagerOpts struct { ngrokAPIKey string region string + + defaultDomainReclaimPolicy string } func apiCmd() *cobra.Command { @@ -167,6 +169,7 @@ func apiCmd() *cobra.Command { c.Flags().StringVar(&opts.bindings.serviceAnnotations, "bindings-service-annotations", "", "Service Annotations to propagate to the target service") c.Flags().StringVar(&opts.bindings.serviceLabels, "bindings-service-labels", "", "Service Labels to propagate to the target service") c.Flags().StringVar(&opts.bindings.ingressEndpoint, "bindings-ingress-endpoint", "", "The endpoint the bindings forwarder connects to") + c.Flags().StringVar(&opts.defaultDomainReclaimPolicy, "default-domain-reclaim-policy", string(ingressv1alpha1.DomainReclaimPolicyDelete), "The default domain reclaim policy to apply to created domains") opts.zapOpts = &zap.Options{} goFlagSet := flag.NewFlagSet("manager", flag.ContinueOnError) @@ -309,6 +312,11 @@ func runOneClickDemoMode(ctx context.Context, mgr ctrl.Manager) error { // runNormalMode runs the operator in normal operation mode func runNormalMode(ctx context.Context, opts apiManagerOpts, k8sClient client.Client, mgr ctrl.Manager, tcpRouteCRDInstalled, tlsRouteCRDInstalled bool) error { + defaultDomainReclaimPolicy, err := validateDomainReclaimPolicy(opts.defaultDomainReclaimPolicy) + if err != nil { + return err + } + ngrokClientset, err := loadNgrokClientset(ctx, opts) if err != nil { return fmt.Errorf("Unable to load ngrokClientSet: %w", err) @@ -324,7 +332,7 @@ func runNormalMode(ctx context.Context, opts apiManagerOpts, k8sClient client.Cl var k8sResourceDriver *managerdriver.Driver if opts.enableFeatureIngress || opts.enableFeatureGateway { // we only need a driver if these features are enabled - k8sResourceDriver, err = getK8sResourceDriver(ctx, mgr, opts, tcpRouteCRDInstalled, tlsRouteCRDInstalled) + k8sResourceDriver, err = getK8sResourceDriver(ctx, mgr, opts, tcpRouteCRDInstalled, tlsRouteCRDInstalled, *defaultDomainReclaimPolicy) if err != nil { return fmt.Errorf("unable to create Driver: %w", err) } @@ -340,7 +348,7 @@ func runNormalMode(ctx context.Context, opts apiManagerOpts, k8sClient client.Cl if opts.enableFeatureIngress { setupLog.Info("Ingress feature set enabled") - if err := enableIngressFeatureSet(ctx, opts, mgr, k8sResourceDriver, ngrokClientset); err != nil { + if err := enableIngressFeatureSet(ctx, opts, mgr, k8sResourceDriver, ngrokClientset, *defaultDomainReclaimPolicy); err != nil { return fmt.Errorf("unable to enable Ingress feature set: %w", err) } } else { @@ -470,13 +478,14 @@ func loadNgrokClientset(ctx context.Context, opts apiManagerOpts) (ngrokapi.Clie } // getK8sResourceDriver returns a new Driver instance that is seeded with the current state of the cluster. -func getK8sResourceDriver(ctx context.Context, mgr manager.Manager, options apiManagerOpts, tcpRouteCRDInstalled, tlsRouteCRDInstalled bool) (*managerdriver.Driver, error) { +func getK8sResourceDriver(ctx context.Context, mgr manager.Manager, options apiManagerOpts, tcpRouteCRDInstalled, tlsRouteCRDInstalled bool, defaultDomainReclaimPolicy ingressv1alpha1.DomainReclaimPolicy) (*managerdriver.Driver, error) { logger := mgr.GetLogger().WithName("cache-store-driver") driverOpts := []managerdriver.DriverOpt{ managerdriver.WithGatewayEnabled(options.enableFeatureGateway), managerdriver.WithClusterDomain(options.clusterDomain), managerdriver.WithDisableGatewayReferenceGrants(options.disableGatewayReferenceGrants), + managerdriver.WithDefaultDomainReclaimPolicy(defaultDomainReclaimPolicy), } if tcpRouteCRDInstalled { @@ -515,7 +524,7 @@ func getK8sResourceDriver(ctx context.Context, mgr manager.Manager, options apiM } // enableIngressFeatureSet enables the Ingress feature set for the operator -func enableIngressFeatureSet(_ context.Context, opts apiManagerOpts, mgr ctrl.Manager, driver *managerdriver.Driver, ngrokClientset ngrokapi.Clientset) error { +func enableIngressFeatureSet(_ context.Context, opts apiManagerOpts, mgr ctrl.Manager, driver *managerdriver.Driver, ngrokClientset ngrokapi.Clientset, defaultDomainReclaimPolicy ingressv1alpha1.DomainReclaimPolicy) error { if err := (&ingresscontroller.IngressReconciler{ Client: mgr.GetClient(), Log: ctrl.Log.WithName("controllers").WithName("ingress"), @@ -623,11 +632,12 @@ func enableIngressFeatureSet(_ context.Context, opts apiManagerOpts, mgr ctrl.Ma } if err := (&ngrokcontroller.CloudEndpointReconciler{ - Client: mgr.GetClient(), - Log: ctrl.Log.WithName("controllers").WithName("cloud-endpoint"), - Scheme: mgr.GetScheme(), - Recorder: mgr.GetEventRecorderFor("cloud-endpoint-controller"), - NgrokClientset: ngrokClientset, + Client: mgr.GetClient(), + Log: ctrl.Log.WithName("controllers").WithName("cloud-endpoint"), + Scheme: mgr.GetScheme(), + Recorder: mgr.GetEventRecorderFor("cloud-endpoint-controller"), + NgrokClientset: ngrokClientset, + DefaultDomainReclaimPolicy: ptr.To(defaultDomainReclaimPolicy), }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "CloudEndpoint") os.Exit(1) diff --git a/cmd/common.go b/cmd/common.go new file mode 100644 index 00000000..e5ff3947 --- /dev/null +++ b/cmd/common.go @@ -0,0 +1,25 @@ +package cmd + +import ( + "fmt" + + ingressv1alpha1 "github.com/ngrok/ngrok-operator/api/ingress/v1alpha1" + "k8s.io/utils/ptr" +) + +func validateDomainReclaimPolicy(policy string) (*ingressv1alpha1.DomainReclaimPolicy, error) { + switch policy { + case string(ingressv1alpha1.DomainReclaimPolicyDelete): + return ptr.To(ingressv1alpha1.DomainReclaimPolicyDelete), nil + case string(ingressv1alpha1.DomainReclaimPolicyRetain): + return ptr.To(ingressv1alpha1.DomainReclaimPolicyRetain), nil + default: + return nil, fmt.Errorf("invalid default domain reclaim policy: %s. Allowed Values are: %v", + policy, + []ingressv1alpha1.DomainReclaimPolicy{ + ingressv1alpha1.DomainReclaimPolicyDelete, + ingressv1alpha1.DomainReclaimPolicyRetain, + }, + ) + } +} diff --git a/helm/ngrok-operator/README.md b/helm/ngrok-operator/README.md index 73e79571..d649bf9b 100644 --- a/helm/ngrok-operator/README.md +++ b/helm/ngrok-operator/README.md @@ -70,31 +70,32 @@ To uninstall the chart: ### Operator Manager parameters -| Name | Description | Value | -| ------------------------------------ | ----------------------------------------------------------------------------------------- | ------- | -| `replicaCount` | The number of controllers to run. | `1` | -| `affinity` | Affinity for the controller pod assignment | `{}` | -| `podAffinityPreset` | Pod affinity preset. Ignored if `affinity` is set. Allowed values: `soft` or `hard` | `""` | -| `podAntiAffinityPreset` | Pod anti-affinity preset. Ignored if `affinity` is set. Allowed values: `soft` or `hard` | `soft` | -| `nodeAffinityPreset.type` | Node affinity preset type. Ignored if `affinity` is set. Allowed values: `soft` or `hard` | `""` | -| `nodeAffinityPreset.key` | Node label key to match. Ignored if `affinity` is set. | `""` | -| `nodeAffinityPreset.values` | Node label values to match. Ignored if `affinity` is set. | `[]` | -| `nodeSelector` | Node labels for manager pod(s) | `{}` | -| `tolerations` | Tolerations for manager pod(s) | `[]` | -| `topologySpreadConstraints` | Topology Spread Constraints for manager pod(s) | `[]` | -| `priorityClassName` | Priority class for pod scheduling | `""` | -| `lifecycle` | an object containing lifecycle configuration | `{}` | -| `podDisruptionBudget.create` | Enable a Pod Disruption Budget creation | `false` | -| `podDisruptionBudget.maxUnavailable` | Maximum number/percentage of pods that may be made unavailable | `""` | -| `podDisruptionBudget.minAvailable` | Minimum number/percentage of pods that should remain scheduled | `""` | -| `resources.limits` | The resources limits for the container | `{}` | -| `resources.requests` | The requested resources for the container | `{}` | -| `extraVolumes` | An array of extra volumes to add to the controller. | `[]` | -| `extraVolumeMounts` | An array of extra volume mounts to add to the controller. | `[]` | -| `extraEnv` | an object of extra environment variables to add to the controller. | `{}` | -| `serviceAccount.create` | Specifies whether a ServiceAccount should be created | `true` | -| `serviceAccount.name` | The name of the ServiceAccount to use. | `""` | -| `serviceAccount.annotations` | Additional annotations to add to the ServiceAccount | `{}` | +| Name | Description | Value | +| ------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------- | -------- | +| `replicaCount` | The number of controllers to run. | `1` | +| `affinity` | Affinity for the controller pod assignment | `{}` | +| `podAffinityPreset` | Pod affinity preset. Ignored if `affinity` is set. Allowed values: `soft` or `hard` | `""` | +| `podAntiAffinityPreset` | Pod anti-affinity preset. Ignored if `affinity` is set. Allowed values: `soft` or `hard` | `soft` | +| `nodeAffinityPreset.type` | Node affinity preset type. Ignored if `affinity` is set. Allowed values: `soft` or `hard` | `""` | +| `nodeAffinityPreset.key` | Node label key to match. Ignored if `affinity` is set. | `""` | +| `nodeAffinityPreset.values` | Node label values to match. Ignored if `affinity` is set. | `[]` | +| `nodeSelector` | Node labels for manager pod(s) | `{}` | +| `tolerations` | Tolerations for manager pod(s) | `[]` | +| `topologySpreadConstraints` | Topology Spread Constraints for manager pod(s) | `[]` | +| `priorityClassName` | Priority class for pod scheduling | `""` | +| `lifecycle` | an object containing lifecycle configuration | `{}` | +| `podDisruptionBudget.create` | Enable a Pod Disruption Budget creation | `false` | +| `podDisruptionBudget.maxUnavailable` | Maximum number/percentage of pods that may be made unavailable | `""` | +| `podDisruptionBudget.minAvailable` | Minimum number/percentage of pods that should remain scheduled | `""` | +| `resources.limits` | The resources limits for the container | `{}` | +| `resources.requests` | The requested resources for the container | `{}` | +| `extraVolumes` | An array of extra volumes to add to the controller. | `[]` | +| `extraVolumeMounts` | An array of extra volume mounts to add to the controller. | `[]` | +| `extraEnv` | an object of extra environment variables to add to the controller. | `{}` | +| `serviceAccount.create` | Specifies whether a ServiceAccount should be created | `true` | +| `serviceAccount.name` | The name of the ServiceAccount to use. | `""` | +| `serviceAccount.annotations` | Additional annotations to add to the ServiceAccount | `{}` | +| `defaultDomainReclaimPolicy` | The default domain reclaim policy to use for domains created by the operator. Valid values are "Delete" and "Retain". The default is "Delete". | `Delete` | ### Logging configuration diff --git a/helm/ngrok-operator/templates/agent/deployment.yaml b/helm/ngrok-operator/templates/agent/deployment.yaml index 53251ff7..5ffed512 100644 --- a/helm/ngrok-operator/templates/agent/deployment.yaml +++ b/helm/ngrok-operator/templates/agent/deployment.yaml @@ -91,6 +91,7 @@ spec: - --health-probe-bind-address=:8081 - --metrics-bind-address=:8080 - --manager-name={{ include "ngrok-operator.fullname" . }}-agent-manager + - --default-domain-reclaim-policy={{ .Values.defaultDomainReclaimPolicy }} securityContext: allowPrivilegeEscalation: false env: diff --git a/helm/ngrok-operator/templates/controller-deployment.yaml b/helm/ngrok-operator/templates/controller-deployment.yaml index 61bb542d..77373013 100644 --- a/helm/ngrok-operator/templates/controller-deployment.yaml +++ b/helm/ngrok-operator/templates/controller-deployment.yaml @@ -72,6 +72,7 @@ spec: args: - api-manager - --release-name={{ .Release.Name }} + - --default-domain-reclaim-policy={{ .Values.defaultDomainReclaimPolicy }} {{- include "ngrok-operator.manager.cliFeatureFlags" . | nindent 8 }} {{- if .Values.oneClickDemoMode }} - --one-click-demo-mode diff --git a/helm/ngrok-operator/tests/__snapshot__/controller-deployment_test.yaml.snap b/helm/ngrok-operator/tests/__snapshot__/controller-deployment_test.yaml.snap index 5eef9a21..dac0d31c 100644 --- a/helm/ngrok-operator/tests/__snapshot__/controller-deployment_test.yaml.snap +++ b/helm/ngrok-operator/tests/__snapshot__/controller-deployment_test.yaml.snap @@ -54,6 +54,7 @@ Should match all-options snapshot: - args: - api-manager - --release-name=RELEASE-NAME + - --default-domain-reclaim-policy=Delete - --enable-feature-ingress=true - --enable-feature-gateway=true - --disable-reference-grants=false @@ -765,6 +766,7 @@ Should match default snapshot: - args: - api-manager - --release-name=RELEASE-NAME + - --default-domain-reclaim-policy=Delete - --enable-feature-ingress=true - --enable-feature-gateway=true - --disable-reference-grants=false diff --git a/helm/ngrok-operator/tests/agent/__snapshot__/deployment_test.yaml.snap b/helm/ngrok-operator/tests/agent/__snapshot__/deployment_test.yaml.snap index 690490bf..f5390794 100644 --- a/helm/ngrok-operator/tests/agent/__snapshot__/deployment_test.yaml.snap +++ b/helm/ngrok-operator/tests/agent/__snapshot__/deployment_test.yaml.snap @@ -60,6 +60,7 @@ Should match snapshot: - --health-probe-bind-address=:8081 - --metrics-bind-address=:8080 - --manager-name=RELEASE-NAME-ngrok-operator-agent-manager + - --default-domain-reclaim-policy=Delete command: - /ngrok-operator env: diff --git a/helm/ngrok-operator/tests/agent/deployment_test.yaml b/helm/ngrok-operator/tests/agent/deployment_test.yaml index 4be2ac78..24e0f2e3 100644 --- a/helm/ngrok-operator/tests/agent/deployment_test.yaml +++ b/helm/ngrok-operator/tests/agent/deployment_test.yaml @@ -90,3 +90,11 @@ tests: - equal: path: spec.template.spec.containers[0].resources value: *resources +- it: Should set the default domain reclaim policy arg + set: + defaultDomainReclaimPolicy: "Retain" + template: agent/deployment.yaml + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: --default-domain-reclaim-policy=Retain diff --git a/helm/ngrok-operator/tests/controller-deployment_test.yaml b/helm/ngrok-operator/tests/controller-deployment_test.yaml index 9ed95c08..b31f4863 100644 --- a/helm/ngrok-operator/tests/controller-deployment_test.yaml +++ b/helm/ngrok-operator/tests/controller-deployment_test.yaml @@ -277,6 +277,15 @@ tests: - contains: path: spec.template.spec.containers[0].args content: --zap-stacktrace-level=error +- it: Should set the default domain reclaim policy arg + set: + defaultDomainReclaimPolicy: "Retain" + template: controller-deployment.yaml + documentIndex: 0 # Document 0 is the deployment since its the first template + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: --default-domain-reclaim-policy=Retain - it: Defaults to having "soft" pod anti-affinity template: controller-deployment.yaml documentIndex: 0 # Document 0 is the deployment since its the first template diff --git a/helm/ngrok-operator/values.schema.json b/helm/ngrok-operator/values.schema.json index bcb93840..f73e6524 100644 --- a/helm/ngrok-operator/values.schema.json +++ b/helm/ngrok-operator/values.schema.json @@ -243,6 +243,11 @@ } } }, + "defaultDomainReclaimPolicy": { + "type": "string", + "description": "The default domain reclaim policy to use for domains created by the operator. Valid values are \"Delete\" and \"Retain\". The default is \"Delete\".", + "default": "Delete" + }, "log": { "type": "object", "properties": { diff --git a/helm/ngrok-operator/values.yaml b/helm/ngrok-operator/values.yaml index ce43b68e..0eb8e1d5 100644 --- a/helm/ngrok-operator/values.yaml +++ b/helm/ngrok-operator/values.yaml @@ -197,6 +197,9 @@ serviceAccount: name: "" annotations: {} +## @param defaultDomainReclaimPolicy The default domain reclaim policy to use for domains created by the operator. Valid values are "Delete" and "Retain". The default is "Delete". +defaultDomainReclaimPolicy: "Delete" + ## ## @section Logging configuration ## diff --git a/internal/controller/agent/agent_endpoint_controller.go b/internal/controller/agent/agent_endpoint_controller.go index c7a6e429..9741bf69 100644 --- a/internal/controller/agent/agent_endpoint_controller.go +++ b/internal/controller/agent/agent_endpoint_controller.go @@ -74,6 +74,8 @@ type AgentEndpointReconciler struct { TunnelDriver *tunneldriver.TunnelDriver controller *controller.BaseController[*ngrokv1alpha1.AgentEndpoint] + + DefaultDomainReclaimPolicy *ingressv1alpha1.DomainReclaimPolicy } // SetupWithManager sets up the controller with the Manager @@ -415,6 +417,11 @@ func (r *AgentEndpointReconciler) ensureDomainExists(ctx context.Context, aep *n Domain: domain, }, } + + if r.DefaultDomainReclaimPolicy != nil { + newDomain.Spec.ReclaimPolicy = *r.DefaultDomainReclaimPolicy + } + if err := r.Create(ctx, newDomain); err != nil { r.Recorder.Event(aep, v1.EventTypeWarning, "DomainCreationFailed", fmt.Sprintf("Failed to create Domain CRD %s", hyphenatedDomain)) return err diff --git a/internal/controller/gateway/httproute_controller_test.go b/internal/controller/gateway/httproute_controller_test.go index 6d8b8173..f23ec4b3 100644 --- a/internal/controller/gateway/httproute_controller_test.go +++ b/internal/controller/gateway/httproute_controller_test.go @@ -132,15 +132,15 @@ var _ = Describe("HTTPRoute controller", Ordered, func() { When("the parent ref is an unsupported type", func() { var ( - service v1.Service + service *v1.Service svcGVK schema.GroupVersionKind ) BeforeEach(func(ctx SpecContext) { var err error service = testutils.NewTestServiceV1(testutils.RandomName("svc"), "default") - Expect(k8sClient.Create(ctx, &service)).To(Succeed()) - svcGVK, err = k8sClient.GroupVersionKindFor(&service) + Expect(k8sClient.Create(ctx, service)).To(Succeed()) + svcGVK, err = k8sClient.GroupVersionKindFor(service) Expect(err).ToNot(HaveOccurred()) route = &gatewayv1.HTTPRoute{ diff --git a/internal/controller/ngrok/cloudendpoint_controller.go b/internal/controller/ngrok/cloudendpoint_controller.go index 607403a7..1eaf5e73 100644 --- a/internal/controller/ngrok/cloudendpoint_controller.go +++ b/internal/controller/ngrok/cloudendpoint_controller.go @@ -63,6 +63,8 @@ type CloudEndpointReconciler struct { Log logr.Logger Recorder record.EventRecorder NgrokClientset ngrokapi.Clientset + + DefaultDomainReclaimPolicy *ingressv1alpha1.DomainReclaimPolicy } // Define a custom error types to catch and handle requeuing logic for @@ -372,6 +374,11 @@ func (r *CloudEndpointReconciler) ensureDomainExists(ctx context.Context, clep * Domain: domain, }, } + + if r.DefaultDomainReclaimPolicy != nil { + newDomain.Spec.ReclaimPolicy = *r.DefaultDomainReclaimPolicy + } + if err := r.Create(ctx, newDomain); err != nil { r.Recorder.Event(clep, v1.EventTypeWarning, "DomainCreationFailed", fmt.Sprintf("Failed to create Domain CRD %s", hyphenatedDomain)) return newDomain, err diff --git a/internal/store/store_test.go b/internal/store/store_test.go index 0141d7f2..cd80240d 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -34,7 +34,7 @@ var _ = Describe("Store", func() { Context("when the ingress class exists", func() { BeforeEach(func() { ic := testutils.NewTestIngressClass(ngrokIngressClass, true, true) - Expect(store.Add(&ic)).To(BeNil()) + Expect(store.Add(ic)).To(BeNil()) }) It("returns the ingress class", func() { ic, err := store.GetIngressClassV1(ngrokIngressClass) @@ -55,7 +55,7 @@ var _ = Describe("Store", func() { Context("when the ingress exists", func() { BeforeEach(func() { ing := testutils.NewTestIngressV1("test-ingress", "test-namespace") - Expect(store.Add(&ing)).To(BeNil()) + Expect(store.Add(ing)).To(BeNil()) }) It("returns the ingress", func() { ing, err := store.GetIngressV1("test-ingress", "test-namespace") @@ -76,7 +76,7 @@ var _ = Describe("Store", func() { Context("when the service exists", func() { BeforeEach(func() { svc := testutils.NewTestServiceV1("test-service", "test-namespace") - Expect(store.Add(&svc)).To(BeNil()) + Expect(store.Add(svc)).To(BeNil()) }) It("returns the service", func() { svc, err := store.GetServiceV1("test-service", "test-namespace") @@ -181,9 +181,9 @@ var _ = Describe("Store", func() { Context("when the ngrok ingress exists", func() { BeforeEach(func() { ing := testutils.NewTestIngressV1WithClass("test-ingress", "test-namespace", ngrokIngressClass) - Expect(store.Add(&ing)).To(BeNil()) + Expect(store.Add(ing)).To(BeNil()) ic := testutils.NewTestIngressClass(ngrokIngressClass, true, true) - Expect(store.Add(&ic)).To(BeNil()) + Expect(store.Add(ic)).To(BeNil()) }) It("returns the ngrok ingress", func() { ing, err := store.GetNgrokIngressV1("test-ingress", "test-namespace") @@ -192,7 +192,7 @@ var _ = Describe("Store", func() { }) It("Filters out ingresses that don't match the ngrok ingress class", func() { ingNotNgrok := testutils.NewTestIngressV1WithClass("ingNotNgrok", "test-namespace", "not-ngrok") - Expect(store.Add(&ingNotNgrok)).To(BeNil()) + Expect(store.Add(ingNotNgrok)).To(BeNil()) ing, err := store.GetNgrokIngressV1("ingNotNgrok", "test-namespace") Expect(err).To(HaveOccurred()) @@ -200,7 +200,7 @@ var _ = Describe("Store", func() { }) It("Filters finds ones without a class if we are default", func() { ingNoClass := testutils.NewTestIngressV1("ingNoClass", "test-namespace") - Expect(store.Add(&ingNoClass)).To(BeNil()) + Expect(store.Add(ingNoClass)).To(BeNil()) ing, err := store.GetNgrokIngressV1("ingNoClass", "test-namespace") Expect(err).ToNot(HaveOccurred()) @@ -220,11 +220,11 @@ var _ = Describe("Store", func() { Context("when there are ngrok ingress classes", func() { BeforeEach(func() { ic1 := testutils.NewTestIngressClass("ngrok1", true, true) - Expect(store.Add(&ic1)).To(BeNil()) + Expect(store.Add(ic1)).To(BeNil()) ic2 := testutils.NewTestIngressClass("ngrok2", true, true) - Expect(store.Add(&ic2)).To(BeNil()) + Expect(store.Add(ic2)).To(BeNil()) ic3 := testutils.NewTestIngressClass("different", true, false) - Expect(store.Add(&ic3)).To(BeNil()) + Expect(store.Add(ic3)).To(BeNil()) }) It("returns the ngrok ingress classes and doesn't return the different one", func() { ics := store.ListNgrokIngressClassesV1() @@ -440,28 +440,28 @@ var _ = Describe("Store", func() { icOtherDefault := testutils.NewTestIngressClass("test", true, false) icOtherNotDefault := testutils.NewTestIngressClass("test", false, false) - var _ = DescribeTable("IngressClassFiltering", func(ingressClasses []netv1.IngressClass, expectedMatchingIngressesCount int) { + var _ = DescribeTable("IngressClassFiltering", func(ingressClasses []*netv1.IngressClass, expectedMatchingIngressesCount int) { iMatching := testutils.NewTestIngressV1WithClass("test1", "test", "ngrok") iNotMatching := testutils.NewTestIngressV1WithClass("test2", "test", "test") iNoClass := testutils.NewTestIngressV1("test3", "test") - Expect(store.Add(&iMatching)).To(BeNil()) - Expect(store.Add(&iNotMatching)).To(BeNil()) - Expect(store.Add(&iNoClass)).To(BeNil()) + Expect(store.Add(iMatching)).To(BeNil()) + Expect(store.Add(iNotMatching)).To(BeNil()) + Expect(store.Add(iNoClass)).To(BeNil()) for _, ic := range ingressClasses { - Expect(store.Add(&ic)).To(BeNil()) + Expect(store.Add(ic)).To(BeNil()) } ings := store.ListNgrokIngressesV1() Expect(len(ings)).To(Equal(expectedMatchingIngressesCount)) }, - Entry("No ingress classes", []netv1.IngressClass{}, 0), - Entry("just us not as default", []netv1.IngressClass{icUsNotDefault}, 1), - Entry("just us as default", []netv1.IngressClass{icUsDefault}, 2), - Entry("just another not as default", []netv1.IngressClass{icOtherNotDefault}, 0), - Entry("just another as default", []netv1.IngressClass{icOtherDefault}, 0), - Entry("us and another neither default", []netv1.IngressClass{icUsNotDefault, icOtherNotDefault}, 1), - Entry("us and another them default", []netv1.IngressClass{icUsNotDefault, icOtherDefault}, 1), - Entry("us and another us default", []netv1.IngressClass{icUsDefault, icOtherNotDefault}, 2), - Entry("us and another both default", []netv1.IngressClass{icUsDefault, icOtherDefault}, 2), + Entry("No ingress classes", []*netv1.IngressClass{}, 0), + Entry("just us not as default", []*netv1.IngressClass{icUsNotDefault}, 1), + Entry("just us as default", []*netv1.IngressClass{icUsDefault}, 2), + Entry("just another not as default", []*netv1.IngressClass{icOtherNotDefault}, 0), + Entry("just another as default", []*netv1.IngressClass{icOtherDefault}, 0), + Entry("us and another neither default", []*netv1.IngressClass{icUsNotDefault, icOtherNotDefault}, 1), + Entry("us and another them default", []*netv1.IngressClass{icUsNotDefault, icOtherDefault}, 1), + Entry("us and another us default", []*netv1.IngressClass{icUsDefault, icOtherNotDefault}, 2), + Entry("us and another both default", []*netv1.IngressClass{icUsDefault, icOtherDefault}, 2), ) }) @@ -533,13 +533,13 @@ var _ = Describe("Store", func() { }) var _ = Describe("Issue #56", func() { - var multiRuleIngress netv1.Ingress + var multiRuleIngress *netv1.Ingress BeforeEach(func() { ngrokClass := testutils.NewTestIngressClass(ngrokIngressClass, true, true) otherClass := testutils.NewTestIngressClass("other", false, false) - Expect(store.Add(&ngrokClass)).ToNot(HaveOccurred()) - Expect(store.Add(&otherClass)).ToNot(HaveOccurred()) + Expect(store.Add(ngrokClass)).ToNot(HaveOccurred()) + Expect(store.Add(otherClass)).ToNot(HaveOccurred()) multiRuleIngress = testutils.NewTestIngressV1WithClass("multi-rule-ingress", "test-namespace", ngrokClass.Name) multiRuleIngress.Spec.Rules = []netv1.IngressRule{ { @@ -598,11 +598,11 @@ var _ = Describe("Store", func() { Context("when an ngrok ingress has multiple rules", func() { It("should not error", func() { - Expect(store.Add(&multiRuleIngress)).ToNot(HaveOccurred()) + Expect(store.Add(multiRuleIngress)).ToNot(HaveOccurred()) }) It("should return the ngrok ingress when queried by name & namespace", func() { - Expect(store.Add(&multiRuleIngress)).ToNot(HaveOccurred()) + Expect(store.Add(multiRuleIngress)).ToNot(HaveOccurred()) ing, err := store.GetNgrokIngressV1(multiRuleIngress.Name, multiRuleIngress.Namespace) Expect(err).ToNot(HaveOccurred()) Expect(ing).ToNot(BeNil()) @@ -610,10 +610,10 @@ var _ = Describe("Store", func() { }) It("should return the ngrok ingress when listed", func() { - Expect(store.Add(&multiRuleIngress)).ToNot(HaveOccurred()) + Expect(store.Add(multiRuleIngress)).ToNot(HaveOccurred()) ings := store.ListNgrokIngressesV1() Expect(ings).To(HaveLen(1)) - Expect(*ings[0]).To(Equal(multiRuleIngress)) + Expect(ings[0]).To(Equal(multiRuleIngress)) }) }) }) @@ -632,7 +632,7 @@ var _ = Describe("Store", func() { log: logger, } ngrokClass := testutils.NewTestIngressClass("ngrok", true, true) - Expect(store.Add(&ngrokClass)).To(BeNil()) + Expect(store.Add(ngrokClass)).To(BeNil()) }) Context("when ingress has missing HTTP rules", func() { @@ -641,7 +641,7 @@ var _ = Describe("Store", func() { ing.Spec.Rules = []netv1.IngressRule{{ Host: "test.com", }} - ok, err := store.shouldHandleIngressIsValid(&ing) + ok, err := store.shouldHandleIngressIsValid(ing) Expect(ok).To(BeFalse()) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("HTTP rules are required for ingress")) @@ -661,7 +661,7 @@ var _ = Describe("Store", func() { Port: netv1.ServiceBackendPort{Number: 80}, }, } - ok, err := store.shouldHandleIngressIsValid(&ing) + ok, err := store.shouldHandleIngressIsValid(ing) Expect(ok).To(BeFalse()) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("Default backends are not supported")) @@ -709,7 +709,7 @@ var _ = Describe("Store", func() { }, }, } - ok, err := store.shouldHandleIngressIsValid(&ing) + ok, err := store.shouldHandleIngressIsValid(ing) Expect(ok).To(BeFalse()) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("A host is required to be set")) @@ -724,7 +724,7 @@ var _ = Describe("Store", func() { ing.Annotations = map[string]string{ "kubernetes.io/ingress.class": "ngrok", } - ok, err := store.shouldHandleIngress(&ing) + ok, err := store.shouldHandleIngress(ing) Expect(ok).To(BeFalse()) Expect(err).ToNot(BeNil()) }) @@ -733,7 +733,7 @@ var _ = Describe("Store", func() { Context("when ingress class does not match", func() { It("returns an error message showing the ingress class name", func() { ing := testutils.NewTestIngressV1WithClass("ingress-wrong-class", "test-namespace", "not-ngrok") - ok, err := store.shouldHandleIngressCheckClass(&ing) + ok, err := store.shouldHandleIngressCheckClass(ing) Expect(ok).To(BeFalse()) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("ingress class mismatching")) @@ -744,7 +744,7 @@ var _ = Describe("Store", func() { Context("when no matching ingress classes are configured", func() { It("lists known ingress classes in the error message", func() { ing := testutils.NewTestIngressV1WithClass("ingress-no-match", "test-namespace", "no-match-class") - ok, err := store.shouldHandleIngressCheckClass(&ing) + ok, err := store.shouldHandleIngressCheckClass(ing) Expect(ok).To(BeFalse()) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("no-match-class")) @@ -756,12 +756,12 @@ var _ = Describe("Store", func() { BeforeEach(func() { // Delete the ngrok ingress class to simulate missing configuration ngrokClass := testutils.NewTestIngressClass("ngrok", true, true) - Expect(store.Delete(&ngrokClass)).To(BeNil()) + Expect(store.Delete(ngrokClass)).To(BeNil()) }) It("emits a warning or event about the missing class", func() { ing := testutils.NewTestIngressV1WithClass("ingress-missing-class", "test-namespace", "ngrok") - ok, err := store.shouldHandleIngressCheckClass(&ing) + ok, err := store.shouldHandleIngressCheckClass(ing) Expect(ok).To(BeFalse()) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("no default ingress class found")) diff --git a/internal/testutils/k8s-resources.go b/internal/testutils/k8s-resources.go index 003609a2..63a17ba9 100644 --- a/internal/testutils/k8s-resources.go +++ b/internal/testutils/k8s-resources.go @@ -17,7 +17,7 @@ import ( gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" ) -func NewTestIngressClass(name string, isDefault bool, isNgrok bool) netv1.IngressClass { +func NewTestIngressClass(name string, isDefault bool, isNgrok bool) *netv1.IngressClass { i := netv1.IngressClass{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -39,17 +39,17 @@ func NewTestIngressClass(name string, isDefault bool, isNgrok bool) netv1.Ingres } } - return i + return &i } -func NewTestIngressV1WithClass(name string, namespace string, ingressClass string) netv1.Ingress { +func NewTestIngressV1WithClass(name string, namespace string, ingressClass string) *netv1.Ingress { i := NewTestIngressV1(name, namespace) i.Spec.IngressClassName = &ingressClass return i } -func NewTestIngressV1(name string, namespace string) netv1.Ingress { - return netv1.Ingress{ +func NewTestIngressV1(name string, namespace string) *netv1.Ingress { + return &netv1.Ingress{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: namespace, @@ -81,8 +81,8 @@ func NewTestIngressV1(name string, namespace string) netv1.Ingress { } } -func NewTestServiceV1(name string, namespace string) corev1.Service { - return corev1.Service{ +func NewTestServiceV1(name string, namespace string) *corev1.Service { + return &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: namespace, @@ -100,10 +100,10 @@ func NewTestServiceV1(name string, namespace string) corev1.Service { } } -func NewDomainV1(name string, namespace string) ingressv1alpha1.Domain { - return ingressv1alpha1.Domain{ +func NewDomainV1(name string, namespace string) *ingressv1alpha1.Domain { + return &ingressv1alpha1.Domain{ ObjectMeta: metav1.ObjectMeta{ - Name: name, + Name: ingressv1alpha1.HyphenatedDomainNameFromURL(name), Namespace: namespace, }, Spec: ingressv1alpha1.DomainSpec{ diff --git a/pkg/managerdriver/domains.go b/pkg/managerdriver/domains.go index 0144d1aa..75d2bfca 100644 --- a/pkg/managerdriver/domains.go +++ b/pkg/managerdriver/domains.go @@ -3,14 +3,15 @@ package managerdriver import ( "context" "fmt" - "reflect" "github.com/go-logr/logr" ingressv1alpha1 "github.com/ngrok/ngrok-operator/api/ingress/v1alpha1" "github.com/ngrok/ngrok-operator/internal/annotations" + "golang.org/x/sync/errgroup" netv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" ) @@ -100,34 +101,40 @@ func gatewayToDomains(log logr.Logger, in *gatewayv1.Gateway, newDomainMetadata } // applyDomains takes a set of the desired domains and current domains, creates any missing desired domains, and updated existing domains if needed -func (d *Driver) applyDomains(ctx context.Context, c client.Client, desiredDomains map[string]ingressv1alpha1.Domain, currentDomains []ingressv1alpha1.Domain) error { +func (d *Driver) applyDomains(ctx context.Context, c client.Client, desiredDomains map[string]ingressv1alpha1.Domain) error { + var g errgroup.Group + for _, desiredDomain := range desiredDomains { - found := false - for _, currDomain := range currentDomains { - if desiredDomain.Name == currDomain.Name && desiredDomain.Namespace == currDomain.Namespace { - // It matches so lets update it if anything is different - if !reflect.DeepEqual(desiredDomain.Spec, currDomain.Spec) { - currDomain.Spec = desiredDomain.Spec - if err := c.Update(ctx, &currDomain); err != nil { - d.log.Error(err, "error updating domain", "domain", desiredDomain) - return err - } - } - found = true - break + g.Go(func() error { + domain := &ingressv1alpha1.Domain{ + ObjectMeta: metav1.ObjectMeta{ + Name: desiredDomain.Name, + Namespace: desiredDomain.Namespace, + }, } - } - if !found { - if err := c.Create(ctx, &desiredDomain); err != nil { - d.log.Error(err, "error creating domain", "domain", desiredDomain) - return err + + res, err := controllerutil.CreateOrPatch(ctx, c, domain, func() error { + domain.Spec.Domain = desiredDomain.Spec.Domain + // Only set the reclaim policy on create + if domain.CreationTimestamp.IsZero() && d.defaultDomainReclaimPolicy != nil { + domain.Spec.ReclaimPolicy = *d.defaultDomainReclaimPolicy + } + return nil + }) + + log := d.log.WithValues("domain", domain.Name, "namespace", domain.Namespace, "result", res) + + if err != nil { + log.Error(err, "error creating or patching domain") + } else { + log.V(3).Info("create or patched domain") } - } - } - // Don't delete domains to prevent accidentally de-registering them and making people re-do DNS + return err + }) + } - return nil + return g.Wait() } // Domain set is a helper data type to encapsulate all of the domains and what sources they are from diff --git a/pkg/managerdriver/driver.go b/pkg/managerdriver/driver.go index d4e1d56b..ae156595 100644 --- a/pkg/managerdriver/driver.go +++ b/pkg/managerdriver/driver.go @@ -61,6 +61,8 @@ type Driver struct { gatewayTCPRouteEnabled bool gatewayTLSRouteEnabled bool disableGatewayReferenceGrants bool + + defaultDomainReclaimPolicy *ingressv1alpha1.DomainReclaimPolicy } type DriverOpt func(*Driver) @@ -101,6 +103,12 @@ func WithGatewayTLSRouteEnabled(enabled bool) DriverOpt { } } +func WithDefaultDomainReclaimPolicy(policy ingressv1alpha1.DomainReclaimPolicy) DriverOpt { + return func(d *Driver) { + d.defaultDomainReclaimPolicy = &policy + } +} + // NewDriver creates a new driver with a basic logger and cache store setup func NewDriver(logger logr.Logger, scheme *runtime.Scheme, controllerName string, managerName types.NamespacedName, opts ...DriverOpt) *Driver { cacheStores := store.NewCacheStores(logger) @@ -580,16 +588,11 @@ func (d *Driver) Sync(ctx context.Context, c client.Client) error { ) translationResult := translator.Translate() - currDomains := &ingressv1alpha1.DomainList{} currEdges := &ingressv1alpha1.HTTPSEdgeList{} currTunnels := &ingressv1alpha1.TunnelList{} currAgentEndpoints := &ngrokv1alpha1.AgentEndpointList{} currCloudEndpoints := &ngrokv1alpha1.CloudEndpointList{} - if err := c.List(ctx, currDomains); err != nil { - d.log.Error(err, "error listing domains") - return err - } if err := c.List(ctx, currEdges, client.MatchingLabels{ labelControllerNamespace: d.managerName.Namespace, labelControllerName: d.managerName.Name, @@ -619,7 +622,7 @@ func (d *Driver) Sync(ctx context.Context, c client.Client) error { return err } - if err := d.applyDomains(ctx, c, domains.totalDomains, currDomains.Items); err != nil { + if err := d.applyDomains(ctx, c, domains.totalDomains); err != nil { return err } diff --git a/pkg/managerdriver/driver_test.go b/pkg/managerdriver/driver_test.go index aab4925f..b939403e 100644 --- a/pkg/managerdriver/driver_test.go +++ b/pkg/managerdriver/driver_test.go @@ -5,7 +5,6 @@ import ( "fmt" "testing" - "github.com/go-logr/logr" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/stretchr/testify/assert" @@ -46,11 +45,10 @@ var _ = Describe("Driver", func() { utilruntime.Must(gatewayv1.Install(scheme)) utilruntime.Must(gatewayv1alpha2.Install(scheme)) utilruntime.Must(ngrokv1alpha1.AddToScheme(scheme)) + BeforeEach(func() { - // create a fake logger to pass into the cachestore - logger := logr.New(logr.Discard().GetSink()) driver = NewDriver( - logger, + GinkgoLogr, scheme, testutils.DefaultControllerName, types.NamespacedName{Name: defaultManagerName}, @@ -73,7 +71,7 @@ var _ = Describe("Driver", func() { d2 := testutils.NewDomainV1("test-domain-2.com", "test-namespace") e1 := testutils.NewHTTPSEdge("test-edge", "test-namespace") e2 := testutils.NewHTTPSEdge("test-edge-2", "test-namespace") - obs := []runtime.Object{&ic1, &ic2, &i1, &i2, &d1, &d2, &e1, &e2} + obs := []runtime.Object{ic1, ic2, i1, i2, d1, d2, &e1, &e2} c := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(obs...).Build() err := driver.Seed(GinkgoT().Context(), c) @@ -92,7 +90,7 @@ var _ = Describe("Driver", func() { Describe("DeleteIngress", func() { It("Should remove the ingress from the store", func() { i1 := testutils.NewTestIngressV1("test-ingress", "test-namespace") - c := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(&i1).Build() + c := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(i1).Build() err := driver.Seed(GinkgoT().Context(), c) Expect(err).ToNot(HaveOccurred()) @@ -102,7 +100,7 @@ var _ = Describe("Driver", func() { }) Expect(err).ToNot(HaveOccurred()) - foundObj, found, err := driver.store.Get(&i1) + foundObj, found, err := driver.store.Get(i1) Expect(err).ToNot(HaveOccurred()) Expect(found).To(BeFalse()) Expect(foundObj).To(BeNil()) @@ -148,7 +146,7 @@ var _ = Describe("Driver", func() { ic1 := testutils.NewTestIngressClass("test-ingress-class", true, true) ic2 := testutils.NewTestIngressClass("test-ingress-class-2", true, true) s := testutils.NewTestServiceV1("example", "test-namespace") - obs := []runtime.Object{&ic1, &ic2, &i1, &i2, &s} + obs := []runtime.Object{ic1, ic2, i1, i2, s} c := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(obs...).Build() for _, obj := range obs { @@ -277,7 +275,7 @@ var _ = Describe("Driver", func() { JustBeforeEach(func() { // Add the services and ingress to the fake client and the store - objs := []runtime.Object{&ic, httpService, httpsService, ingress} + objs := []runtime.Object{ic, httpService, httpsService, ingress} c = fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(objs...).Build() for _, obj := range objs { @@ -472,7 +470,7 @@ var _ = Describe("Driver", func() { JustBeforeEach(func() { // Add the services and ingress to the fake client and the store - objs := []runtime.Object{&ic, trafficPolicy, httpService, ingress} + objs := []runtime.Object{ic, trafficPolicy, httpService, ingress} c = fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(objs...).Build() for _, obj := range objs { @@ -564,11 +562,106 @@ var _ = Describe("Driver", func() { }) }) }) + + When("The defaultDomainReclaimPolicy is set", func() { + var ( + defaultDomainReclaimPolicy ingressv1alpha1.DomainReclaimPolicy + objs []runtime.Object + c client.WithWatch + ) + + BeforeEach(func() { + objs = []runtime.Object{ + testutils.NewTestIngressClass("test-ingress-class", true, true), + testutils.NewTestIngressV1("test-ingress", "test-namespace"), + testutils.NewTestServiceV1("example", "test-namespace"), + } + }) + + JustBeforeEach(func(ctx SpecContext) { + driver = NewDriver( + GinkgoLogr, + scheme, + testutils.DefaultControllerName, + types.NamespacedName{Name: defaultManagerName}, + WithGatewayEnabled(false), + WithSyncAllowConcurrent(true), + WithDefaultDomainReclaimPolicy(defaultDomainReclaimPolicy), + ) + c = fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(objs...).Build() + Expect(driver.Seed(ctx, c)).To(Succeed()) + Expect(driver.Sync(ctx, c)).To(Succeed()) + }) + + When("policy is Delete", func() { + BeforeEach(func() { + defaultDomainReclaimPolicy = ingressv1alpha1.DomainReclaimPolicyDelete + }) + + When("no domains exist", func() { + It("should create new domains with ReclaimPolicy set to Delete", func(ctx SpecContext) { + domains := &ingressv1alpha1.DomainList{} + Expect(c.List(ctx, domains)).To(Succeed()) + Expect(domains.Items).To(HaveLen(1)) + Expect(domains.Items[0].Spec.ReclaimPolicy).To(Equal(ingressv1alpha1.DomainReclaimPolicyDelete)) + }) + }) + + When("a domain already exists", func() { + BeforeEach(func() { + d := testutils.NewDomainV1("example.com", "test-namespace") + d.ObjectMeta.SetCreationTimestamp(metav1.Now()) + d.Spec.ReclaimPolicy = ingressv1alpha1.DomainReclaimPolicyRetain + + objs = append(objs, d) + }) + + It("should not modify the reclaim policy of existing domains", func(ctx SpecContext) { + domains := &ingressv1alpha1.DomainList{} + Expect(c.List(ctx, domains)).To(Succeed()) + Expect(domains.Items).To(HaveLen(1)) + Expect(domains.Items[0].Spec.ReclaimPolicy).To(Equal(ingressv1alpha1.DomainReclaimPolicyRetain)) + }) + }) + }) + + When("policy is Retain", func() { + BeforeEach(func() { + defaultDomainReclaimPolicy = ingressv1alpha1.DomainReclaimPolicyRetain + }) + + When("no domains exist", func() { + It("should create new domains with ReclaimPolicy set to Retain", func(ctx SpecContext) { + domains := &ingressv1alpha1.DomainList{} + Expect(c.List(ctx, domains)).To(Succeed()) + Expect(domains.Items).To(HaveLen(1)) + Expect(domains.Items[0].Spec.ReclaimPolicy).To(Equal(ingressv1alpha1.DomainReclaimPolicyRetain)) + }) + }) + + When("a domain already exists", func() { + BeforeEach(func() { + d := testutils.NewDomainV1("example.com", "test-namespace") + d.ObjectMeta.SetCreationTimestamp(metav1.Now()) + d.Spec.ReclaimPolicy = ingressv1alpha1.DomainReclaimPolicyDelete + + objs = append(objs, d) + }) + + It("should not modify the reclaim policy of existing domains", func(ctx SpecContext) { + domains := &ingressv1alpha1.DomainList{} + Expect(c.List(ctx, domains)).To(Succeed()) + Expect(domains.Items).To(HaveLen(1)) + Expect(domains.Items[0].Spec.ReclaimPolicy).To(Equal(ingressv1alpha1.DomainReclaimPolicyDelete)) + }) + }) + }) + }) }) Describe("calculateIngressLoadBalancerIPStatus", func() { var domains []ingressv1alpha1.Domain - var ingress netv1.Ingress + var ingress *netv1.Ingress var c client.WithWatch var status []netv1.IngressLoadBalancerIngress @@ -583,7 +676,7 @@ var _ = Describe("Driver", func() { Build() domainsByDomain, err := getDomainsByDomain(GinkgoT().Context(), c) Expect(err).ToNot(HaveOccurred()) - status = calculateIngressLoadBalancerIPStatus(&ingress, domainsByDomain) + status = calculateIngressLoadBalancerIPStatus(ingress, domainsByDomain) }) addIngressHostname := func(i *netv1.Ingress, hostname string) { @@ -724,8 +817,8 @@ var _ = Describe("Driver", func() { BeforeEach(func() { ingress = testutils.NewTestIngressV1("test-ingress", "test-namespace") - addIngressHostname(&ingress, "test-domain1.com") - addIngressHostname(&ingress, "test-domain2.com") + addIngressHostname(ingress, "test-domain1.com") + addIngressHostname(ingress, "test-domain2.com") domains = newTestDomainList( newTestDomain( "test-domain1-com", @@ -754,8 +847,8 @@ var _ = Describe("Driver", func() { BeforeEach(func() { ingress = testutils.NewTestIngressV1("test-ingress", "test-namespace") - addIngressHostname(&ingress, "test-domain1.com") - addIngressHostname(&ingress, "test-domain1.com") + addIngressHostname(ingress, "test-domain1.com") + addIngressHostname(ingress, "test-domain1.com") domains = newTestDomainList( newTestDomain( "test-domain1-com", @@ -816,9 +909,9 @@ var _ = Describe("Driver", func() { It("Should return an empty module set if the ingress has no modules annotaion", func() { ing := testutils.NewTestIngressV1("test-ingress", "test") - Expect(driver.store.Add(&ing)).To(BeNil()) + Expect(driver.store.Add(ing)).To(BeNil()) - ms, err := getNgrokModuleSetForObject(&ing, driver.store) + ms, err := getNgrokModuleSetForObject(ing, driver.store) Expect(err).To(BeNil()) Expect(ms.Modules.Compression).To(BeNil()) Expect(ms.Modules.Headers).To(BeNil()) @@ -830,9 +923,9 @@ var _ = Describe("Driver", func() { It("Should return the matching module set if the ingress has a modules annotaion", func() { ing := testutils.NewTestIngressV1("test-ingress", "test") ing.SetAnnotations(map[string]string{"k8s.ngrok.com/modules": "ms1"}) - Expect(driver.store.Add(&ing)).To(BeNil()) + Expect(driver.store.Add(ing)).To(BeNil()) - ms, err := getNgrokModuleSetForObject(&ing, driver.store) + ms, err := getNgrokModuleSetForObject(ing, driver.store) Expect(err).To(BeNil()) Expect(ms.Modules).To(Equal(ms1.Modules)) }) @@ -840,9 +933,9 @@ var _ = Describe("Driver", func() { It("merges modules with the last one winning if multiple module sets are specified", func() { ing := testutils.NewTestIngressV1("test-ingress", "test") ing.SetAnnotations(map[string]string{"k8s.ngrok.com/modules": "ms1,ms2,ms3"}) - Expect(driver.store.Add(&ing)).To(BeNil()) + Expect(driver.store.Add(ing)).To(BeNil()) - ms, err := getNgrokModuleSetForObject(&ing, driver.store) + ms, err := getNgrokModuleSetForObject(ing, driver.store) Expect(err).To(BeNil()) Expect(ms.Modules).To(Equal( ingressv1alpha1.NgrokModuleSetModules{ @@ -1166,7 +1259,7 @@ var _ = Describe("Driver", func() { }, } - obs := []runtime.Object{&ic1, &i1, &i2} + obs := []runtime.Object{ic1, i1, i2} c := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(obs...).Build() Expect(driver.Seed(GinkgoT().Context(), c)).To(BeNil())