Skip to content

feat: Add a default domain reclaim policy #656

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions cmd/agent-manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@
// agent(tunnel driver) flags
region string
rootCAs string

defaultDomainReclaimPolicy string
}

func agentCmd() *cobra.Command {
Expand Down Expand Up @@ -107,6 +109,8 @@
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")

Check warning on line 113 in cmd/agent-manager.go

View check run for this annotation

Codecov / codecov/patch

cmd/agent-manager.go#L112-L113

Added lines #L112 - L113 were not covered by tests
opts.zapOpts = &zap.Options{}
goFlagSet := flag.NewFlagSet("manager", flag.ContinueOnError)
opts.zapOpts.BindFlags(goFlagSet)
Expand All @@ -118,6 +122,11 @@
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
}

Check warning on line 128 in cmd/agent-manager.go

View check run for this annotation

Codecov / codecov/patch

cmd/agent-manager.go#L125-L128

Added lines #L125 - L128 were not covered by tests

buildInfo := version.Get()
setupLog.Info("starting agent-manager", "version", buildInfo.Version, "commit", buildInfo.GitCommit)

Expand Down Expand Up @@ -180,11 +189,12 @@
}

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,

Check warning on line 197 in cmd/agent-manager.go

View check run for this annotation

Codecov / codecov/patch

cmd/agent-manager.go#L192-L197

Added lines #L192 - L197 were not covered by tests
}).SetupWithManager(mgr); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "AgentEndpoint")
os.Exit(1)
Expand Down
28 changes: 19 additions & 9 deletions cmd/api-manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@
ngrokAPIKey string

region string

defaultDomainReclaimPolicy string
}

func apiCmd() *cobra.Command {
Expand Down Expand Up @@ -167,6 +169,7 @@
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")

Check warning on line 172 in cmd/api-manager.go

View check run for this annotation

Codecov / codecov/patch

cmd/api-manager.go#L172

Added line #L172 was not covered by tests

opts.zapOpts = &zap.Options{}
goFlagSet := flag.NewFlagSet("manager", flag.ContinueOnError)
Expand Down Expand Up @@ -309,6 +312,11 @@

// 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
}

Check warning on line 318 in cmd/api-manager.go

View check run for this annotation

Codecov / codecov/patch

cmd/api-manager.go#L315-L318

Added lines #L315 - L318 were not covered by tests

ngrokClientset, err := loadNgrokClientset(ctx, opts)
if err != nil {
return fmt.Errorf("Unable to load ngrokClientSet: %w", err)
Expand All @@ -324,7 +332,7 @@
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)

Check warning on line 335 in cmd/api-manager.go

View check run for this annotation

Codecov / codecov/patch

cmd/api-manager.go#L335

Added line #L335 was not covered by tests
if err != nil {
return fmt.Errorf("unable to create Driver: %w", err)
}
Expand All @@ -340,7 +348,7 @@

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 {

Check warning on line 351 in cmd/api-manager.go

View check run for this annotation

Codecov / codecov/patch

cmd/api-manager.go#L351

Added line #L351 was not covered by tests
return fmt.Errorf("unable to enable Ingress feature set: %w", err)
}
} else {
Expand Down Expand Up @@ -470,13 +478,14 @@
}

// 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) {

Check warning on line 481 in cmd/api-manager.go

View check run for this annotation

Codecov / codecov/patch

cmd/api-manager.go#L481

Added line #L481 was not covered by tests
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),

Check warning on line 488 in cmd/api-manager.go

View check run for this annotation

Codecov / codecov/patch

cmd/api-manager.go#L488

Added line #L488 was not covered by tests
}

if tcpRouteCRDInstalled {
Expand Down Expand Up @@ -515,7 +524,7 @@
}

// 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 {

Check warning on line 527 in cmd/api-manager.go

View check run for this annotation

Codecov / codecov/patch

cmd/api-manager.go#L527

Added line #L527 was not covered by tests
if err := (&ingresscontroller.IngressReconciler{
Client: mgr.GetClient(),
Log: ctrl.Log.WithName("controllers").WithName("ingress"),
Expand Down Expand Up @@ -623,11 +632,12 @@
}

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),

Check warning on line 640 in cmd/api-manager.go

View check run for this annotation

Codecov / codecov/patch

cmd/api-manager.go#L635-L640

Added lines #L635 - L640 were not covered by tests
}).SetupWithManager(mgr); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "CloudEndpoint")
os.Exit(1)
Expand Down
25 changes: 25 additions & 0 deletions cmd/common.go
Original file line number Diff line number Diff line change
@@ -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,
},
)

Check warning on line 23 in cmd/common.go

View check run for this annotation

Codecov / codecov/patch

cmd/common.go#L10-L23

Added lines #L10 - L23 were not covered by tests
}
}
51 changes: 26 additions & 25 deletions helm/ngrok-operator/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions helm/ngrok-operator/templates/agent/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions helm/ngrok-operator/templates/controller-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions helm/ngrok-operator/tests/agent/deployment_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
9 changes: 9 additions & 0 deletions helm/ngrok-operator/tests/controller-deployment_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading