Skip to content

Commit d642636

Browse files
jonstacksjonstacks
authored
feat: Add a default domain reclaim policy (#656)
1 parent 92864af commit d642636

20 files changed

+338
-145
lines changed

‎cmd/agent-manager.go

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,8 @@ type agentManagerOpts struct {
7979
// agent(tunnel driver) flags
8080
region string
8181
rootCAs string
82+
83+
defaultDomainReclaimPolicy string
8284
}
8385

8486
func agentCmd() *cobra.Command {
@@ -107,6 +109,8 @@ func agentCmd() *cobra.Command {
107109
c.Flags().BoolVar(&opts.disableGatewayReferenceGrants, "disable-reference-grants", false, "Opts-out of requiring ReferenceGrants for cross namespace references in Gateway API config")
108110
c.Flags().BoolVar(&opts.enableFeatureBindings, "enable-feature-bindings", false, "Enables the Endpoint Bindings controller")
109111

112+
c.Flags().StringVar(&opts.defaultDomainReclaimPolicy, "default-domain-reclaim-policy", string(ingressv1alpha1.DomainReclaimPolicyDelete), "The default domain reclaim policy to apply to created domains")
113+
110114
opts.zapOpts = &zap.Options{}
111115
goFlagSet := flag.NewFlagSet("manager", flag.ContinueOnError)
112116
opts.zapOpts.BindFlags(goFlagSet)
@@ -118,6 +122,11 @@ func agentCmd() *cobra.Command {
118122
func runAgentController(ctx context.Context, opts agentManagerOpts) error {
119123
ctrl.SetLogger(zap.New(zap.UseFlagOptions(opts.zapOpts)))
120124

125+
defaultDomainReclaimPolicy, err := validateDomainReclaimPolicy(opts.defaultDomainReclaimPolicy)
126+
if err != nil {
127+
return err
128+
}
129+
121130
buildInfo := version.Get()
122131
setupLog.Info("starting agent-manager", "version", buildInfo.Version, "commit", buildInfo.GitCommit)
123132

@@ -180,11 +189,12 @@ func runAgentController(ctx context.Context, opts agentManagerOpts) error {
180189
}
181190

182191
if err = (&agentcontroller.AgentEndpointReconciler{
183-
Client: mgr.GetClient(),
184-
Log: ctrl.Log.WithName("controllers").WithName("agentendpoint"),
185-
Scheme: mgr.GetScheme(),
186-
Recorder: mgr.GetEventRecorderFor("agentendpoint-controller"),
187-
TunnelDriver: td,
192+
Client: mgr.GetClient(),
193+
Log: ctrl.Log.WithName("controllers").WithName("agentendpoint"),
194+
Scheme: mgr.GetScheme(),
195+
Recorder: mgr.GetEventRecorderFor("agentendpoint-controller"),
196+
TunnelDriver: td,
197+
DefaultDomainReclaimPolicy: defaultDomainReclaimPolicy,
188198
}).SetupWithManager(mgr); err != nil {
189199
setupLog.Error(err, "unable to create controller", "controller", "AgentEndpoint")
190200
os.Exit(1)

‎cmd/api-manager.go

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,8 @@ type apiManagerOpts struct {
130130
ngrokAPIKey string
131131

132132
region string
133+
134+
defaultDomainReclaimPolicy string
133135
}
134136

135137
func apiCmd() *cobra.Command {
@@ -167,6 +169,7 @@ func apiCmd() *cobra.Command {
167169
c.Flags().StringVar(&opts.bindings.serviceAnnotations, "bindings-service-annotations", "", "Service Annotations to propagate to the target service")
168170
c.Flags().StringVar(&opts.bindings.serviceLabels, "bindings-service-labels", "", "Service Labels to propagate to the target service")
169171
c.Flags().StringVar(&opts.bindings.ingressEndpoint, "bindings-ingress-endpoint", "", "The endpoint the bindings forwarder connects to")
172+
c.Flags().StringVar(&opts.defaultDomainReclaimPolicy, "default-domain-reclaim-policy", string(ingressv1alpha1.DomainReclaimPolicyDelete), "The default domain reclaim policy to apply to created domains")
170173

171174
opts.zapOpts = &zap.Options{}
172175
goFlagSet := flag.NewFlagSet("manager", flag.ContinueOnError)
@@ -309,6 +312,11 @@ func runOneClickDemoMode(ctx context.Context, mgr ctrl.Manager) error {
309312

310313
// runNormalMode runs the operator in normal operation mode
311314
func runNormalMode(ctx context.Context, opts apiManagerOpts, k8sClient client.Client, mgr ctrl.Manager, tcpRouteCRDInstalled, tlsRouteCRDInstalled bool) error {
315+
defaultDomainReclaimPolicy, err := validateDomainReclaimPolicy(opts.defaultDomainReclaimPolicy)
316+
if err != nil {
317+
return err
318+
}
319+
312320
ngrokClientset, err := loadNgrokClientset(ctx, opts)
313321
if err != nil {
314322
return fmt.Errorf("Unable to load ngrokClientSet: %w", err)
@@ -324,7 +332,7 @@ func runNormalMode(ctx context.Context, opts apiManagerOpts, k8sClient client.Cl
324332
var k8sResourceDriver *managerdriver.Driver
325333
if opts.enableFeatureIngress || opts.enableFeatureGateway {
326334
// we only need a driver if these features are enabled
327-
k8sResourceDriver, err = getK8sResourceDriver(ctx, mgr, opts, tcpRouteCRDInstalled, tlsRouteCRDInstalled)
335+
k8sResourceDriver, err = getK8sResourceDriver(ctx, mgr, opts, tcpRouteCRDInstalled, tlsRouteCRDInstalled, *defaultDomainReclaimPolicy)
328336
if err != nil {
329337
return fmt.Errorf("unable to create Driver: %w", err)
330338
}
@@ -340,7 +348,7 @@ func runNormalMode(ctx context.Context, opts apiManagerOpts, k8sClient client.Cl
340348

341349
if opts.enableFeatureIngress {
342350
setupLog.Info("Ingress feature set enabled")
343-
if err := enableIngressFeatureSet(ctx, opts, mgr, k8sResourceDriver, ngrokClientset); err != nil {
351+
if err := enableIngressFeatureSet(ctx, opts, mgr, k8sResourceDriver, ngrokClientset, *defaultDomainReclaimPolicy); err != nil {
344352
return fmt.Errorf("unable to enable Ingress feature set: %w", err)
345353
}
346354
} else {
@@ -470,13 +478,14 @@ func loadNgrokClientset(ctx context.Context, opts apiManagerOpts) (ngrokapi.Clie
470478
}
471479

472480
// getK8sResourceDriver returns a new Driver instance that is seeded with the current state of the cluster.
473-
func getK8sResourceDriver(ctx context.Context, mgr manager.Manager, options apiManagerOpts, tcpRouteCRDInstalled, tlsRouteCRDInstalled bool) (*managerdriver.Driver, error) {
481+
func getK8sResourceDriver(ctx context.Context, mgr manager.Manager, options apiManagerOpts, tcpRouteCRDInstalled, tlsRouteCRDInstalled bool, defaultDomainReclaimPolicy ingressv1alpha1.DomainReclaimPolicy) (*managerdriver.Driver, error) {
474482
logger := mgr.GetLogger().WithName("cache-store-driver")
475483

476484
driverOpts := []managerdriver.DriverOpt{
477485
managerdriver.WithGatewayEnabled(options.enableFeatureGateway),
478486
managerdriver.WithClusterDomain(options.clusterDomain),
479487
managerdriver.WithDisableGatewayReferenceGrants(options.disableGatewayReferenceGrants),
488+
managerdriver.WithDefaultDomainReclaimPolicy(defaultDomainReclaimPolicy),
480489
}
481490

482491
if tcpRouteCRDInstalled {
@@ -515,7 +524,7 @@ func getK8sResourceDriver(ctx context.Context, mgr manager.Manager, options apiM
515524
}
516525

517526
// enableIngressFeatureSet enables the Ingress feature set for the operator
518-
func enableIngressFeatureSet(_ context.Context, opts apiManagerOpts, mgr ctrl.Manager, driver *managerdriver.Driver, ngrokClientset ngrokapi.Clientset) error {
527+
func enableIngressFeatureSet(_ context.Context, opts apiManagerOpts, mgr ctrl.Manager, driver *managerdriver.Driver, ngrokClientset ngrokapi.Clientset, defaultDomainReclaimPolicy ingressv1alpha1.DomainReclaimPolicy) error {
519528
if err := (&ingresscontroller.IngressReconciler{
520529
Client: mgr.GetClient(),
521530
Log: ctrl.Log.WithName("controllers").WithName("ingress"),
@@ -623,11 +632,12 @@ func enableIngressFeatureSet(_ context.Context, opts apiManagerOpts, mgr ctrl.Ma
623632
}
624633

625634
if err := (&ngrokcontroller.CloudEndpointReconciler{
626-
Client: mgr.GetClient(),
627-
Log: ctrl.Log.WithName("controllers").WithName("cloud-endpoint"),
628-
Scheme: mgr.GetScheme(),
629-
Recorder: mgr.GetEventRecorderFor("cloud-endpoint-controller"),
630-
NgrokClientset: ngrokClientset,
635+
Client: mgr.GetClient(),
636+
Log: ctrl.Log.WithName("controllers").WithName("cloud-endpoint"),
637+
Scheme: mgr.GetScheme(),
638+
Recorder: mgr.GetEventRecorderFor("cloud-endpoint-controller"),
639+
NgrokClientset: ngrokClientset,
640+
DefaultDomainReclaimPolicy: ptr.To(defaultDomainReclaimPolicy),
631641
}).SetupWithManager(mgr); err != nil {
632642
setupLog.Error(err, "unable to create controller", "controller", "CloudEndpoint")
633643
os.Exit(1)

‎cmd/common.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
package cmd
2+
3+
import (
4+
"fmt"
5+
6+
ingressv1alpha1 "github.com/ngrok/ngrok-operator/api/ingress/v1alpha1"
7+
"k8s.io/utils/ptr"
8+
)
9+
10+
func validateDomainReclaimPolicy(policy string) (*ingressv1alpha1.DomainReclaimPolicy, error) {
11+
switch policy {
12+
case string(ingressv1alpha1.DomainReclaimPolicyDelete):
13+
return ptr.To(ingressv1alpha1.DomainReclaimPolicyDelete), nil
14+
case string(ingressv1alpha1.DomainReclaimPolicyRetain):
15+
return ptr.To(ingressv1alpha1.DomainReclaimPolicyRetain), nil
16+
default:
17+
return nil, fmt.Errorf("invalid default domain reclaim policy: %s. Allowed Values are: %v",
18+
policy,
19+
[]ingressv1alpha1.DomainReclaimPolicy{
20+
ingressv1alpha1.DomainReclaimPolicyDelete,
21+
ingressv1alpha1.DomainReclaimPolicyRetain,
22+
},
23+
)
24+
}
25+
}

‎helm/ngrok-operator/README.md

Lines changed: 26 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -70,31 +70,32 @@ To uninstall the chart:
7070

7171
### Operator Manager parameters
7272

73-
| Name | Description | Value |
74-
| ------------------------------------ | ----------------------------------------------------------------------------------------- | ------- |
75-
| `replicaCount` | The number of controllers to run. | `1` |
76-
| `affinity` | Affinity for the controller pod assignment | `{}` |
77-
| `podAffinityPreset` | Pod affinity preset. Ignored if `affinity` is set. Allowed values: `soft` or `hard` | `""` |
78-
| `podAntiAffinityPreset` | Pod anti-affinity preset. Ignored if `affinity` is set. Allowed values: `soft` or `hard` | `soft` |
79-
| `nodeAffinityPreset.type` | Node affinity preset type. Ignored if `affinity` is set. Allowed values: `soft` or `hard` | `""` |
80-
| `nodeAffinityPreset.key` | Node label key to match. Ignored if `affinity` is set. | `""` |
81-
| `nodeAffinityPreset.values` | Node label values to match. Ignored if `affinity` is set. | `[]` |
82-
| `nodeSelector` | Node labels for manager pod(s) | `{}` |
83-
| `tolerations` | Tolerations for manager pod(s) | `[]` |
84-
| `topologySpreadConstraints` | Topology Spread Constraints for manager pod(s) | `[]` |
85-
| `priorityClassName` | Priority class for pod scheduling | `""` |
86-
| `lifecycle` | an object containing lifecycle configuration | `{}` |
87-
| `podDisruptionBudget.create` | Enable a Pod Disruption Budget creation | `false` |
88-
| `podDisruptionBudget.maxUnavailable` | Maximum number/percentage of pods that may be made unavailable | `""` |
89-
| `podDisruptionBudget.minAvailable` | Minimum number/percentage of pods that should remain scheduled | `""` |
90-
| `resources.limits` | The resources limits for the container | `{}` |
91-
| `resources.requests` | The requested resources for the container | `{}` |
92-
| `extraVolumes` | An array of extra volumes to add to the controller. | `[]` |
93-
| `extraVolumeMounts` | An array of extra volume mounts to add to the controller. | `[]` |
94-
| `extraEnv` | an object of extra environment variables to add to the controller. | `{}` |
95-
| `serviceAccount.create` | Specifies whether a ServiceAccount should be created | `true` |
96-
| `serviceAccount.name` | The name of the ServiceAccount to use. | `""` |
97-
| `serviceAccount.annotations` | Additional annotations to add to the ServiceAccount | `{}` |
73+
| Name | Description | Value |
74+
| ------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------- | -------- |
75+
| `replicaCount` | The number of controllers to run. | `1` |
76+
| `affinity` | Affinity for the controller pod assignment | `{}` |
77+
| `podAffinityPreset` | Pod affinity preset. Ignored if `affinity` is set. Allowed values: `soft` or `hard` | `""` |
78+
| `podAntiAffinityPreset` | Pod anti-affinity preset. Ignored if `affinity` is set. Allowed values: `soft` or `hard` | `soft` |
79+
| `nodeAffinityPreset.type` | Node affinity preset type. Ignored if `affinity` is set. Allowed values: `soft` or `hard` | `""` |
80+
| `nodeAffinityPreset.key` | Node label key to match. Ignored if `affinity` is set. | `""` |
81+
| `nodeAffinityPreset.values` | Node label values to match. Ignored if `affinity` is set. | `[]` |
82+
| `nodeSelector` | Node labels for manager pod(s) | `{}` |
83+
| `tolerations` | Tolerations for manager pod(s) | `[]` |
84+
| `topologySpreadConstraints` | Topology Spread Constraints for manager pod(s) | `[]` |
85+
| `priorityClassName` | Priority class for pod scheduling | `""` |
86+
| `lifecycle` | an object containing lifecycle configuration | `{}` |
87+
| `podDisruptionBudget.create` | Enable a Pod Disruption Budget creation | `false` |
88+
| `podDisruptionBudget.maxUnavailable` | Maximum number/percentage of pods that may be made unavailable | `""` |
89+
| `podDisruptionBudget.minAvailable` | Minimum number/percentage of pods that should remain scheduled | `""` |
90+
| `resources.limits` | The resources limits for the container | `{}` |
91+
| `resources.requests` | The requested resources for the container | `{}` |
92+
| `extraVolumes` | An array of extra volumes to add to the controller. | `[]` |
93+
| `extraVolumeMounts` | An array of extra volume mounts to add to the controller. | `[]` |
94+
| `extraEnv` | an object of extra environment variables to add to the controller. | `{}` |
95+
| `serviceAccount.create` | Specifies whether a ServiceAccount should be created | `true` |
96+
| `serviceAccount.name` | The name of the ServiceAccount to use. | `""` |
97+
| `serviceAccount.annotations` | Additional annotations to add to the ServiceAccount | `{}` |
98+
| `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` |
9899

99100
### Logging configuration
100101

‎helm/ngrok-operator/templates/agent/deployment.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ spec:
9191
- --health-probe-bind-address=:8081
9292
- --metrics-bind-address=:8080
9393
- --manager-name={{ include "ngrok-operator.fullname" . }}-agent-manager
94+
- --default-domain-reclaim-policy={{ .Values.defaultDomainReclaimPolicy }}
9495
securityContext:
9596
allowPrivilegeEscalation: false
9697
env:

‎helm/ngrok-operator/templates/controller-deployment.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ spec:
7272
args:
7373
- api-manager
7474
- --release-name={{ .Release.Name }}
75+
- --default-domain-reclaim-policy={{ .Values.defaultDomainReclaimPolicy }}
7576
{{- include "ngrok-operator.manager.cliFeatureFlags" . | nindent 8 }}
7677
{{- if .Values.oneClickDemoMode }}
7778
- --one-click-demo-mode

‎helm/ngrok-operator/tests/__snapshot__/controller-deployment_test.yaml.snap

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎helm/ngrok-operator/tests/agent/__snapshot__/deployment_test.yaml.snap

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎helm/ngrok-operator/tests/agent/deployment_test.yaml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,3 +90,11 @@ tests:
9090
- equal:
9191
path: spec.template.spec.containers[0].resources
9292
value: *resources
93+
- it: Should set the default domain reclaim policy arg
94+
set:
95+
defaultDomainReclaimPolicy: "Retain"
96+
template: agent/deployment.yaml
97+
asserts:
98+
- contains:
99+
path: spec.template.spec.containers[0].args
100+
content: --default-domain-reclaim-policy=Retain

‎helm/ngrok-operator/tests/controller-deployment_test.yaml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,15 @@ tests:
277277
- contains:
278278
path: spec.template.spec.containers[0].args
279279
content: --zap-stacktrace-level=error
280+
- it: Should set the default domain reclaim policy arg
281+
set:
282+
defaultDomainReclaimPolicy: "Retain"
283+
template: controller-deployment.yaml
284+
documentIndex: 0 # Document 0 is the deployment since its the first template
285+
asserts:
286+
- contains:
287+
path: spec.template.spec.containers[0].args
288+
content: --default-domain-reclaim-policy=Retain
280289
- it: Defaults to having "soft" pod anti-affinity
281290
template: controller-deployment.yaml
282291
documentIndex: 0 # Document 0 is the deployment since its the first template

0 commit comments

Comments
 (0)