From 7e1c74c61fd60a62af6a50c692bf717acaced3cd Mon Sep 17 00:00:00 2001 From: reoring Date: Mon, 2 Nov 2026 09:28:40 +0900 Subject: [PATCH] route53: ZoneClass negativeCacheTTLSeconds owns the hosted zone SOA MINIMUM Resolvers cache NXDOMAIN/NODATA for the SOA MINIMUM (RFC 2308); Route 53 creates zones with 86400, so records written by dns-api can stay invisible to in-VPC resolvers for a day. The Route 53 ZoneClass parameter negativeCacheTTLSeconds makes the controller converge the apex SOA MINIMUM of created and adopted hosted zones (one read per reconcile, one UPSERT changing only MINIMUM, tracked as a pending hosted-zone change NegativeCacheTTL). The observed MINIMUM is published in zone status data. --- README.md | 1 + .../config/provider/route53_extensions.yaml | 12 ++ .../dns-api/templates/provider-route53.yaml | 12 ++ docs/design/route53-provider.md | 20 +- .../route53/controllers/zoneclass/helpers.go | 3 + .../zoneclass/negative_cache_ttl_test.go | 137 ++++++++++++ .../controllers/zoneunit/aws_provider.go | 1 + .../route53/controllers/zoneunit/soa.go | 80 +++++++ .../zoneunit/soa_lifecycle_test.go | 86 ++++++++ .../route53/controllers/zoneunit/soa_test.go | 195 ++++++++++++++++++ .../route53/controllers/zoneunit/zone.go | 5 + .../controllers/zoneunit/zone_status.go | 8 +- .../zoneunit/zone_test_fixtures_test.go | 28 ++- .../api/route53/v1alpha1/route53zone_types.go | 6 + .../v1alpha1/route53zoneclass_types.go | 7 + pkg/go/api/route53/v1alpha1/scheme_test.go | 13 ++ .../route53/v1alpha1/zz_generated.deepcopy.go | 5 + 17 files changed, 606 insertions(+), 13 deletions(-) create mode 100644 internal/go/providers/route53/controllers/zoneclass/negative_cache_ttl_test.go create mode 100644 internal/go/providers/route53/controllers/zoneunit/soa.go create mode 100644 internal/go/providers/route53/controllers/zoneunit/soa_lifecycle_test.go create mode 100644 internal/go/providers/route53/controllers/zoneunit/soa_test.go diff --git a/README.md b/README.md index 5c29add..ac2f090 100644 --- a/README.md +++ b/README.md @@ -7,6 +7,7 @@ It is useful when platform teams want to give application teams a safe DNS workf dns-api is currently focused on: - Amazon Route 53 public hosted zones, and adoption of existing private hosted zones +- Optional Route 53 negative-cache TTL management (SOA MINIMUM) for created and adopted hosted zones - Cloudflare full zones - External verification of customer-managed DNS (opt-in, credentialless) - [Hosted authoritative DNS](./docs/design/hosted-provider.md) served from Kubernetes snapshots (opt-in, credentialless) diff --git a/app/operator/config/provider/route53_extensions.yaml b/app/operator/config/provider/route53_extensions.yaml index 0aa412a..2fb2fc5 100644 --- a/app/operator/config/provider/route53_extensions.yaml +++ b/app/operator/config/provider/route53_extensions.yaml @@ -51,6 +51,12 @@ spec: enum: - Allow - Deny + negativeCacheTTLSeconds: + type: integer + format: int32 + minimum: 0 + maximum: 2147483647 + description: Sets the SOA MINIMUM (RFC 2308 negative caching TTL) of hosted zones created or adopted by this class. Absent leaves the SOA untouched. tags: type: object description: Additional Route 53 tags applied to hosted zones created by this ZoneClass. @@ -93,6 +99,12 @@ spec: enum: - Public - Private + negativeCacheTTLSeconds: + type: integer + format: int32 + minimum: 0 + maximum: 2147483647 + description: Last observed SOA MINIMUM when managed by the ZoneClass; zero is omitted. recordSet: supportedTypes: - A diff --git a/deploy/charts/dns-api/templates/provider-route53.yaml b/deploy/charts/dns-api/templates/provider-route53.yaml index 5bbc032..dc847b1 100644 --- a/deploy/charts/dns-api/templates/provider-route53.yaml +++ b/deploy/charts/dns-api/templates/provider-route53.yaml @@ -52,6 +52,12 @@ spec: enum: - Allow - Deny + negativeCacheTTLSeconds: + type: integer + format: int32 + minimum: 0 + maximum: 2147483647 + description: Sets the SOA MINIMUM (RFC 2308 negative caching TTL) of hosted zones created or adopted by this class. Absent leaves the SOA untouched. tags: type: object description: Additional Route 53 tags applied to hosted zones created by this ZoneClass. @@ -94,6 +100,12 @@ spec: enum: - Public - Private + negativeCacheTTLSeconds: + type: integer + format: int32 + minimum: 0 + maximum: 2147483647 + description: Last observed SOA MINIMUM when managed by the ZoneClass; zero is omitted. recordSet: supportedTypes: - A diff --git a/docs/design/route53-provider.md b/docs/design/route53-provider.md index f2a7c1d..f4fff22 100644 --- a/docs/design/route53-provider.md +++ b/docs/design/route53-provider.md @@ -117,6 +117,7 @@ Route 53 `ZoneClass.spec.parameters` fields: - `zoneCreationPolicy`: whether new public hosted zone creation is allowed. Values: `Create` or `Deny`. Default: `Create` for Public, `Deny` for Private. Private permits only absent or `Deny`. - `zoneDeletionPolicy`: whether the hosted zone is deleted when the `Zone` is deleted. Values: `Delete` or `Retain`. Default: `Retain`. Private permits only absent or `Retain`. - `sameNameZonePolicy`: how to handle existing same-name hosted zones in the Route 53 account. Values: `Allow` or `Deny`. Default: `Deny`. +- `negativeCacheTTLSeconds`: optional SOA MINIMUM (negative caching TTL), integer `0..2147483647` seconds. Applies to created and adopted public or private zones; absent leaves the SOA untouched. - `tags`: Route 53 tags added to created hosted zones. Route 53 `Provider` schema descriptions for `zoneClass.schemas.parameters`: @@ -127,11 +128,26 @@ Route 53 `Provider` schema descriptions for `zoneClass.schemas.parameters`: | `zoneCreationPolicy` | `Controls whether dns-api may create a Route 53 public hosted zone when a Zone does not adopt an existing hosted zone. Defaults to Create for Public and Deny for Private.` | | `zoneDeletionPolicy` | `Controls whether dns-api deletes the Route 53 hosted zone when the Kubernetes Zone is deleted. Private requires Retain.` | | `sameNameZonePolicy` | `Controls whether dns-api may create a new Route 53 hosted zone when another same-name hosted zone already exists.` | +| `negativeCacheTTLSeconds` | `Sets the SOA MINIMUM (RFC 2308 negative caching TTL) of hosted zones created or adopted by this class. Absent leaves the SOA untouched.` | | `tags` | `Additional Route 53 tags applied to hosted zones created by this ZoneClass.` | | `tags.` | `One Route 53 tag value applied to hosted zones created by this ZoneClass.` | AWS credentials themselves are not stored in `ZoneClass.spec.parameters`. `ZoneClass.spec.identityRef` selects the AWS account and assume role chain through `Route53Identity`. +### Negative cache TTL + +Resolvers cache NXDOMAIN and NODATA answers using the SOA MINIMUM (bounded by the SOA RRset TTL under RFC 2308). A low negative-cache TTL lets records written by dns-api become visible promptly after a previously cached negative answer expires. Set the platform ZoneClass parameter, for example: + +```yaml +spec: + parameters: + negativeCacheTTLSeconds: 60 +``` + +When set, dns-api owns only the apex SOA MINIMUM, for both created and adopted hosted zones, including private zones. It reads the SOA once on each normal zone observation and silently corrects drift with one UPSERT, preserving the RRset TTL and MNAME, RNAME, SERIAL, REFRESH, RETRY, and EXPIRE. Omitting the parameter performs no SOA read or write and does not restore an earlier value. Already cached negative answers retain their original expiration; lowering MINIMUM does not flush resolver caches. + +The submitted change is stored in `ZoneUnit.status.provider.state.pendingHostedZoneChange` with operation `NegativeCacheTTL`. Like other pending hosted-zone changes, it blocks child RecordSet reconciliation with `Programmed=False`, reason `ProviderChangePending`, until the existing change poll observes `INSYNC`, clears the pending state, and requeues for fresh observation. Public `negativeCacheTTLSeconds` is the last observed MINIMUM, not the submitted desired value; zero is omitted. Provider errors use the normal provider-error classification. A missing or malformed SOA sets `Programmed=False`, reason `ExternalResourceMismatch`, without exposing its raw value. + ### Private hosted zones With `zoneType: Private`, dns-api adopts an existing private hosted zone through `Zone.spec.adoption.hostedZoneId` or `Zone.spec.adoption.byName: true` and manages its RecordSets. dns-api never creates or deletes private hosted zones and never manages their VPC associations. Create the private hosted zone and configure its VPC associations outside dns-api before adoption. @@ -228,6 +244,7 @@ Route 53 `Zone.status.provider.data` fields: - `hostedZoneID`: Route 53 hosted zone ID in `Z...` form. - `zoneType`: observed Route 53 hosted zone type, `Public` or `Private`. +- `negativeCacheTTLSeconds`: last observed SOA MINIMUM when managed by the ZoneClass, in seconds. Zero is omitted; while a change is pending this remains the previous observed value. Route 53 `Provider` schema descriptions for `zone.schemas.statusProviderData`: @@ -235,6 +252,7 @@ Route 53 `Provider` schema descriptions for `zone.schemas.statusProviderData`: | --- | --- | | `hostedZoneID` | `Route 53 hosted zone ID managed for this Zone.` | | `zoneType` | `Observed Route 53 hosted zone type.` | +| `negativeCacheTTLSeconds` | `Last observed SOA MINIMUM when managed by the ZoneClass; zero is omitted.` | Pending Route 53 hosted-zone and record-set changes are stored in `ZoneUnit.status.provider.state`, not in public claim status. Pending fields are kept only while Route 53 reports `PENDING`. Completed `INSYNC` changes are not retained as history. `Programmed=True` is set after pending fields are removed and provider state is re-observed to match `ZoneUnit.spec`. @@ -245,7 +263,7 @@ Route 53-specific Zone Events: | type | reason | related | When emitted | | --- | --- | --- | --- | | `Normal` | `Route53HostedZoneChangeSubmitted` | none | `CreateHostedZone` or `DeleteHostedZone` change submitted. | -| `Normal` | `Route53HostedZoneChangeInSync` | none | Route 53 hosted zone create/delete change becomes `INSYNC`. | +| `Normal` | `Route53HostedZoneChangeInSync` | none | A hosted-zone change, including `NegativeCacheTTL`, becomes `INSYNC`. | ## Route 53 RecordSet Provider Schema diff --git a/internal/go/providers/route53/controllers/zoneclass/helpers.go b/internal/go/providers/route53/controllers/zoneclass/helpers.go index 285f3a7..4dddcdb 100644 --- a/internal/go/providers/route53/controllers/zoneclass/helpers.go +++ b/internal/go/providers/route53/controllers/zoneclass/helpers.go @@ -77,6 +77,9 @@ func route53ZoneClassParameters(zoneClass *dnsv1alpha1.ZoneClass) (*route53v1alp if err := json.Unmarshal(zoneClass.Spec.Parameters.Raw, ¶ms); err != nil { return nil, fmt.Errorf("parameters must match Route 53 ZoneClass schema: %w", err) } + if params.NegativeCacheTTLSeconds != nil && *params.NegativeCacheTTLSeconds < 0 { + return nil, errors.New("negativeCacheTTLSeconds must be between 0 and 2147483647") + } return ¶ms, nil } diff --git a/internal/go/providers/route53/controllers/zoneclass/negative_cache_ttl_test.go b/internal/go/providers/route53/controllers/zoneclass/negative_cache_ttl_test.go new file mode 100644 index 0000000..2462034 --- /dev/null +++ b/internal/go/providers/route53/controllers/zoneclass/negative_cache_ttl_test.go @@ -0,0 +1,137 @@ +package route53 + +import ( + "bytes" + "context" + "encoding/json" + "os" + "path/filepath" + "reflect" + "testing" + "text/template" + + corewebhook "github.com/appthrust/dns-api/internal/go/core/webhook" + dnsv1alpha1 "github.com/appthrust/dns-api/pkg/go/api/dns/v1alpha1" + admissionv1 "k8s.io/api/admission/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + "sigs.k8s.io/yaml" +) + +func TestNegativeCacheTTLProviderSchemas(t *testing.T) { + root := filepath.Join("..", "..", "..", "..", "..", "..") + providers := map[string]*dnsv1alpha1.Provider{} + for name, path := range map[string]string{ + "built-in": "app/operator/config/provider/route53_extensions.yaml", + "chart": "deploy/charts/dns-api/templates/provider-route53.yaml", + } { + data, err := os.ReadFile(filepath.Join(root, path)) + if err != nil { + t.Fatal(err) + } + tmpl, err := template.New(name).Parse(string(data)) + if err != nil { + t.Fatal(err) + } + var rendered bytes.Buffer + if err := tmpl.Execute(&rendered, map[string]any{ + "Values": map[string]any{"providers": map[string]any{"route53": map[string]any{"enabled": true}}}, + }); err != nil { + t.Fatal(err) + } + provider := &dnsv1alpha1.Provider{} + if err := yaml.Unmarshal(rendered.Bytes(), provider); err != nil { + t.Fatal(err) + } + providers[name] = provider + } + builtIn, chart := providers["built-in"].Spec.Versions[0], providers["chart"].Spec.Versions[0] + if !reflect.DeepEqual(builtIn.ZoneClass.Schemas.Parameters, chart.ZoneClass.Schemas.Parameters) || + !reflect.DeepEqual(builtIn.Zone.Schemas.StatusProviderData, chart.Zone.Schemas.StatusProviderData) { + t.Fatal("built-in and chart ZoneClass parameters or Zone status schemas differ") + } + for name, provider := range providers { + t.Run(name, func(t *testing.T) { + t.Run("observed status", func(t *testing.T) { + scheme := testScheme(t) + k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(provider.DeepCopy()).Build() + validator := corewebhook.NewCoreValidator(k8sClient, scheme) + for _, value := range []string{"0", "60", "2147483647", "-1", "2147483648"} { + zone := &dnsv1alpha1.Zone{ + TypeMeta: metav1.TypeMeta{APIVersion: "dns.appthrust.io/v1alpha1", Kind: "Zone"}, + Spec: dnsv1alpha1.ZoneSpec{Provider: dnsv1alpha1.ProviderReference{Name: provider.Name, Version: "v1alpha1"}}, + Status: dnsv1alpha1.ZoneStatus{Provider: &dnsv1alpha1.ProviderStatus{ + Data: runtime.RawExtension{Raw: []byte(`{"negativeCacheTTLSeconds":` + value + `}`)}, + }}, + } + body, err := json.Marshal(zone) + if err != nil { + t.Fatal(err) + } + response := validator.Handle(context.Background(), admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{ + Kind: metav1.GroupVersionKind{Group: "dns.appthrust.io", Version: "v1alpha1", Kind: "Zone"}, + Operation: admissionv1.Update, SubResource: "status", + Object: runtime.RawExtension{Raw: body}, + }}) + valid := value != "-1" && value != "2147483648" + if response.Allowed != valid { + t.Fatalf("status %s admission = %v, want %v; result %#v", value, response.Allowed, valid, response.Result) + } + } + }) + for _, test := range []struct { + name string + parameters string + valid bool + }{ + {name: "absent", parameters: `{}`, valid: true}, + {name: "sixty", parameters: `{"negativeCacheTTLSeconds":60}`, valid: true}, + {name: "zero", parameters: `{"negativeCacheTTLSeconds":0}`, valid: true}, + {name: "maximum", parameters: `{"negativeCacheTTLSeconds":2147483647}`, valid: true}, + {name: "private", parameters: `{"zoneType":"Private","negativeCacheTTLSeconds":60}`, valid: true}, + {name: "negative", parameters: `{"negativeCacheTTLSeconds":-1}`}, + {name: "above maximum", parameters: `{"negativeCacheTTLSeconds":2147483648}`}, + {name: "fractional", parameters: `{"negativeCacheTTLSeconds":0.5}`}, + {name: "string", parameters: `{"negativeCacheTTLSeconds":"60"}`}, + } { + t.Run(test.name, func(t *testing.T) { + ctx := context.Background() + class := route53ZoneClass("platform", "route53", nil) + class.TypeMeta = metav1.TypeMeta{APIVersion: "dns.appthrust.io/v1alpha1", Kind: "ZoneClass"} + class.Spec.Parameters = runtime.RawExtension{Raw: []byte(test.parameters)} + scheme := testScheme(t) + k8sClient := fake.NewClientBuilder().WithScheme(scheme). + WithObjects(provider.DeepCopy(), class, acceptedRoute53Identity("platform", "route53-dev")). + WithStatusSubresource(&dnsv1alpha1.ZoneClass{}).Build() + body, err := json.Marshal(class) + if err != nil { + t.Fatal(err) + } + response := corewebhook.NewCoreValidator(k8sClient, scheme).Handle(ctx, admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{ + Kind: metav1.GroupVersionKind{Group: "dns.appthrust.io", Version: "v1alpha1", Kind: "ZoneClass"}, + Operation: admissionv1.Create, + Object: runtime.RawExtension{Raw: body}, + }}) + if response.Allowed != test.valid { + t.Fatalf("admission allowed = %v, want %v; result %#v", response.Allowed, test.valid, response.Result) + } + if _, err := (&ZoneClassReconciler{Client: k8sClient}).Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(class)}); err != nil { + t.Fatal(err) + } + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(class), class); err != nil { + t.Fatal(err) + } + if test.valid { + assertCondition(t, class.Status.Conditions, string(dnsv1alpha1.ConditionAccepted), metav1.ConditionTrue, "Accepted") + } else { + assertCondition(t, class.Status.Conditions, string(dnsv1alpha1.ConditionAccepted), metav1.ConditionFalse, "InvalidProviderPayload") + } + }) + } + }) + } +} diff --git a/internal/go/providers/route53/controllers/zoneunit/aws_provider.go b/internal/go/providers/route53/controllers/zoneunit/aws_provider.go index 9b6c7d1..288c381 100644 --- a/internal/go/providers/route53/controllers/zoneunit/aws_provider.go +++ b/internal/go/providers/route53/controllers/zoneunit/aws_provider.go @@ -56,6 +56,7 @@ type Provider interface { GetChange(ctx context.Context, id string) (*route53v1alpha1.Route53Change, error) DeleteHostedZone(ctx context.Context, id string) (*route53v1alpha1.Route53Change, error) TagHostedZone(ctx context.Context, id string, tags map[string]string) error + GetRecordSet(ctx context.Context, hostedZoneID, recordName string, recordType dnsv1alpha1.RecordType) (*RecordSetResource, error) ListRecordSets(ctx context.Context, hostedZoneID string) ([]RecordSetResource, error) ChangeRecordSets(ctx context.Context, hostedZoneID string, changes []RecordSetChange) (*route53v1alpha1.Route53Change, error) } diff --git a/internal/go/providers/route53/controllers/zoneunit/soa.go b/internal/go/providers/route53/controllers/zoneunit/soa.go new file mode 100644 index 0000000..cf53397 --- /dev/null +++ b/internal/go/providers/route53/controllers/zoneunit/soa.go @@ -0,0 +1,80 @@ +package route53 + +import ( + "context" + "errors" + "strconv" + "strings" + + dnsv1alpha1 "github.com/appthrust/dns-api/pkg/go/api/dns/v1alpha1" + route53v1alpha1 "github.com/appthrust/dns-api/pkg/go/api/route53/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" +) + +// SOA is provider-owned zone configuration, not a supported RecordSet claim type. +const route53RecordTypeSOA dnsv1alpha1.RecordType = "SOA" + +type soaRecord struct { + fields []string + minimum int32 +} + +func parseSOA(value string) (soaRecord, error) { + fields := strings.Fields(value) + if len(fields) != 7 { + return soaRecord{}, errors.New("Route 53 hosted zone apex SOA must contain exactly seven fields") + } + minimum, err := strconv.ParseInt(fields[6], 10, 32) + if err != nil || minimum < 0 { + return soaRecord{}, errors.New("Route 53 hosted zone apex SOA MINIMUM must be between 0 and 2147483647") + } + return soaRecord{fields: fields, minimum: int32(minimum)}, nil +} + +func (soa soaRecord) withMinimum(minimum int32) string { + soa.fields[6] = strconv.FormatInt(int64(minimum), 10) + return strings.Join(soa.fields, " ") +} + +func (r *ZoneReconciler) reconcileNegativeCacheTTL(ctx context.Context, provider Provider, zone *dnsv1alpha1.Zone, hostedZone HostedZone, params *route53v1alpha1.Route53ZoneClassParameters) (ctrl.Result, bool, error) { + if params.NegativeCacheTTLSeconds == nil { + return ctrl.Result{}, false, nil + } + record, err := provider.GetRecordSet(ctx, hostedZone.ID, hostedZone.Name, route53RecordTypeSOA) + if err != nil { + result, err := r.failProgrammedForProviderError(ctx, zone, err) + return result, true, err + } + if record == nil || record.TTL == nil || len(record.Values) != 1 { + return ctrl.Result{}, true, r.setProgrammed(ctx, zone, metav1.ConditionFalse, "ExternalResourceMismatch", "Route 53 hosted zone apex SOA must contain exactly one value and a TTL") + } + soa, err := parseSOA(record.Values[0]) + if err != nil { + return ctrl.Result{}, true, r.setProgrammed(ctx, zone, metav1.ConditionFalse, "ExternalResourceMismatch", err.Error()) + } + if err := r.patchRoute53ZoneStatus(ctx, zone, func(data *route53v1alpha1.Route53ZoneStatusData) { + data.NegativeCacheTTLSeconds = soa.minimum + }); err != nil { + return ctrl.Result{}, true, err + } + if soa.minimum == *params.NegativeCacheTTLSeconds { + return ctrl.Result{}, false, nil + } + + // Preserve the RRset TTL and all six other SOA fields, including SERIAL. + record.Values[0] = soa.withMinimum(*params.NegativeCacheTTLSeconds) + change, err := provider.ChangeRecordSets(ctx, hostedZone.ID, []RecordSetChange{{Action: RecordSetChangeActionUpsert, RecordSet: *record}}) + if err != nil { + result, err := r.failProgrammedForProviderError(ctx, zone, err) + return result, true, err + } + if err := r.patchRoute53ZoneStatus(ctx, zone, func(data *route53v1alpha1.Route53ZoneStatusData) { + data.PendingHostedZoneChange = pendingChangeFromChange(change, "NegativeCacheTTL") + }); err != nil { + return ctrl.Result{}, true, err + } + // Use the existing hosted-zone change barrier. Publish only observed MINIMUM, + // and re-read the SOA after INSYNC rather than reporting the submitted value. + return ctrl.Result{RequeueAfter: r.changeCheckAfter()}, true, r.setProgrammed(ctx, zone, metav1.ConditionFalse, "ProviderChangePending", "Route 53 hosted zone change is pending") +} diff --git a/internal/go/providers/route53/controllers/zoneunit/soa_lifecycle_test.go b/internal/go/providers/route53/controllers/zoneunit/soa_lifecycle_test.go new file mode 100644 index 0000000..4682312 --- /dev/null +++ b/internal/go/providers/route53/controllers/zoneunit/soa_lifecycle_test.go @@ -0,0 +1,86 @@ +package route53 + +import ( + "context" + "testing" + + dnsv1alpha1 "github.com/appthrust/dns-api/pkg/go/api/dns/v1alpha1" + route53v1alpha1 "github.com/appthrust/dns-api/pkg/go/api/route53/v1alpha1" + "github.com/aws/smithy-go" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func TestZoneReconcilerCreatedZoneNegativeCacheTTL(t *testing.T) { + ctx := context.Background() + provider := newFakeProvider() + class := route53ZoneClass("app", "route53", nil) + class.Spec.Parameters = runtime.RawExtension{Raw: []byte(`{"negativeCacheTTLSeconds":60}`)} + unit := route53ZoneUnit("app", "apps-example-com", "app", class.Name) + k8sClient := fake.NewClientBuilder().WithScheme(testScheme(t)). + WithObjects(route53Provider(), class, acceptedRoute53Identity("app", "route53-dev"), unit). + WithStatusSubresource(&dnsv1alpha1.ZoneUnit{}).Build() + reconciler := &ZoneReconciler{Client: k8sClient, Provider: provider} + request := ctrl.Request{NamespacedName: client.ObjectKeyFromObject(unit)} + if _, err := reconciler.Reconcile(ctx, request); err != nil { + t.Fatal(err) + } + state := mustZoneUnitStatusData(t, ctx, k8sClient, unit.Namespace, unit.Name) + if len(provider.created) != 1 || state.PendingHostedZoneChange == nil { + t.Fatalf("created zones = %d, state = %#v", len(provider.created), state) + } + provider.changes[state.PendingHostedZoneChange.ID].Status = route53v1alpha1.Route53ChangeStatusInSync + soa := RecordSetResource{HostedZoneID: state.HostedZoneID, Name: "apps.example.com.", Type: "SOA", TTL: ptr(int64(900)), Values: []string{defaultSOA}} + provider.records[recordKey(soa.HostedZoneID, soa.Name, soa.Type)] = soa + for range 2 { + if _, err := reconciler.Reconcile(ctx, request); err != nil { + t.Fatal(err) + } + } + if len(provider.created) != 1 || len(provider.upserted) != 1 || provider.upserted[0].HostedZoneID != state.HostedZoneID { + t.Fatalf("created-zone SOA not converged: created=%d, upserted=%#v", len(provider.created), provider.upserted) + } + state = mustZoneUnitStatusData(t, ctx, k8sClient, unit.Namespace, unit.Name) + if state.PendingHostedZoneChange == nil || state.PendingHostedZoneChange.Operation != "NegativeCacheTTL" { + t.Fatalf("SOA change not tracked: %#v", state) + } +} + +func TestZoneReconcilerNegativeCacheTTLProviderErrors(t *testing.T) { + for _, operation := range []string{"read", "write"} { + t.Run(operation, func(t *testing.T) { + ctx := context.Background() + provider := newFakeProvider() + provider.zones["ZSOA"] = HostedZone{ID: "ZSOA", Name: "apps.example.com"} + soa := RecordSetResource{HostedZoneID: "ZSOA", Name: "apps.example.com.", Type: "SOA", TTL: ptr(int64(900)), Values: []string{defaultSOA}} + provider.records[recordKey(soa.HostedZoneID, soa.Name, soa.Type)] = soa + denied := &smithy.GenericAPIError{Code: "AccessDenied", Message: "private provider diagnostic"} + if operation == "read" { + provider.getRecordSetErr = denied + } else { + provider.changeRecordSetsErr = func(string, []RecordSetChange) error { return denied } + } + class := route53ZoneClass("app", "route53", nil) + class.Spec.Parameters = runtime.RawExtension{Raw: []byte(`{"negativeCacheTTLSeconds":60}`)} + unit := route53ZoneUnit("app", "apps-example-com", "app", class.Name) + unit.Spec.Zone.Adoption = runtime.RawExtension{Raw: []byte(`{"hostedZoneId":"ZSOA"}`)} + k8sClient := fake.NewClientBuilder().WithScheme(testScheme(t)). + WithObjects(route53Provider(), class, acceptedRoute53Identity("app", "route53-dev"), unit). + WithStatusSubresource(&dnsv1alpha1.ZoneUnit{}).Build() + reconciler := &ZoneReconciler{Client: k8sClient, Provider: provider} + if _, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(unit)}); err != nil { + t.Fatal(err) + } + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(unit), unit); err != nil { + t.Fatal(err) + } + assertCondition(t, unit.Status.Zone.Conditions, string(dnsv1alpha1.ConditionProgrammed), metav1.ConditionFalse, "ProviderAccessDenied") + if len(provider.upserted) != 0 { + t.Fatal("failed provider operation wrote SOA") + } + }) + } +} diff --git a/internal/go/providers/route53/controllers/zoneunit/soa_test.go b/internal/go/providers/route53/controllers/zoneunit/soa_test.go new file mode 100644 index 0000000..c27f24f --- /dev/null +++ b/internal/go/providers/route53/controllers/zoneunit/soa_test.go @@ -0,0 +1,195 @@ +package route53 + +import ( + "context" + "encoding/json" + "reflect" + "strconv" + "strings" + "testing" + + dnsv1alpha1 "github.com/appthrust/dns-api/pkg/go/api/dns/v1alpha1" + route53v1alpha1 "github.com/appthrust/dns-api/pkg/go/api/route53/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +const defaultSOA = "ns-1.awsdns-00.com. awsdns-hostmaster.amazon.com. 1 7200 900 1209600 86400" + +func TestZoneReconcilerNegativeCacheTTL(t *testing.T) { + for _, test := range []struct { + name string + parameters string + value string + adopt bool + private bool + wantWrites int + wantTTL int32 + malformed bool + }{ + {name: "absent", parameters: `{}`, value: defaultSOA}, + {name: "managed differs", parameters: `{"negativeCacheTTLSeconds":60}`, value: defaultSOA, wantWrites: 1, wantTTL: 60}, + {name: "already equal", parameters: `{"negativeCacheTTLSeconds":86400}`, value: defaultSOA, wantTTL: 86400}, + {name: "adopted public", parameters: `{"negativeCacheTTLSeconds":60}`, value: defaultSOA, adopt: true, wantWrites: 1, wantTTL: 60}, + {name: "adopted private", parameters: `{"zoneType":"Private","negativeCacheTTLSeconds":60}`, value: defaultSOA, adopt: true, private: true, wantWrites: 1, wantTTL: 60}, + {name: "zero", parameters: `{"negativeCacheTTLSeconds":0}`, value: defaultSOA, wantWrites: 1}, + {name: "maximum", parameters: `{"negativeCacheTTLSeconds":2147483647}`, value: defaultSOA, wantWrites: 1, wantTTL: 2147483647}, + {name: "malformed", parameters: `{"negativeCacheTTLSeconds":60}`, value: "private malformed value", malformed: true}, + } { + t.Run(test.name, func(t *testing.T) { + ctx := context.Background() + provider := newFakeProvider() + hostedZone := HostedZone{ID: "ZSOA", Name: "apps.example.com", Private: test.private} + provider.zones[hostedZone.ID] = hostedZone + original := RecordSetResource{HostedZoneID: hostedZone.ID, Name: hostedZone.Name + ".", Type: "SOA", TTL: ptr(int64(900)), Values: []string{test.value}} + key := recordKey(original.HostedZoneID, original.Name, original.Type) + provider.records[key] = original + class := route53ZoneClass("app", "route53", nil) + class.Spec.Parameters = runtime.RawExtension{Raw: []byte(test.parameters)} + unit := route53ZoneUnit("app", "apps-example-com", "app", class.Name) + if test.adopt { + unit.Spec.Zone.Adoption = runtime.RawExtension{Raw: []byte(`{"hostedZoneId":"ZSOA"}`)} + } else { + unit.Status.Provider = &dnsv1alpha1.ProviderStatus{State: runtime.RawExtension{Raw: []byte(`{"hostedZoneID":"ZSOA"}`)}} + } + k8sClient := fake.NewClientBuilder().WithScheme(testScheme(t)). + WithObjects(route53Provider(), class, acceptedRoute53Identity("app", "route53-dev"), unit). + WithStatusSubresource(&dnsv1alpha1.ZoneUnit{}, &dnsv1alpha1.ZoneClass{}, &route53v1alpha1.Route53Identity{}).Build() + reconciler := &ZoneReconciler{Client: k8sClient, Provider: provider} + request := ctrl.Request{NamespacedName: client.ObjectKeyFromObject(unit)} + reconcile := func() { + t.Helper() + if _, err := reconciler.Reconcile(ctx, request); err != nil { + t.Fatal(err) + } + if err := k8sClient.Get(ctx, request.NamespacedName, unit); err != nil { + t.Fatal(err) + } + } + reconcile() + wantReads := 1 + if test.parameters == `{}` { + wantReads = 0 + } + if len(provider.readRecords) != wantReads || (wantReads == 1 && provider.readRecords[0] != key) { + t.Fatalf("SOA reads = %v, want %d apex reads", provider.readRecords, wantReads) + } + if len(provider.upserted) != test.wantWrites { + t.Fatalf("SOA writes = %d, want %d", len(provider.upserted), test.wantWrites) + } + if test.malformed { + condition := assertCondition(t, unit.Status.Zone.Conditions, string(dnsv1alpha1.ConditionProgrammed), metav1.ConditionFalse, "ExternalResourceMismatch") + if condition.Message != "Route 53 hosted zone apex SOA must contain exactly seven fields" { + t.Fatalf("unsafe or unexpected SOA message: %q", condition.Message) + } + return + } + if test.wantWrites != 0 { + state := mustZoneUnitStatusData(t, ctx, k8sClient, unit.Namespace, unit.Name) + pending := state.PendingHostedZoneChange + if pending == nil || pending.Operation != "NegativeCacheTTL" || pending.Status != route53v1alpha1.Route53ChangeStatusPending { + t.Fatalf("pending change = %#v", pending) + } + assertCondition(t, unit.Status.Zone.Conditions, string(dnsv1alpha1.ConditionProgrammed), metav1.ConditionFalse, "ProviderChangePending") + var observed struct { + Minimum int32 `json:"negativeCacheTTLSeconds"` + } + if err := json.Unmarshal(unit.Status.Zone.Provider.Data.Raw, &observed); err != nil || observed.Minimum != 86400 { + t.Fatalf("submitted change must retain observed MINIMUM 86400: %#v, %v", observed, err) + } + reconcile() + if len(provider.upserted) != 1 { + t.Fatal("pending SOA change was resubmitted") + } + if len(provider.readRecords) != 1 || len(provider.listedRecordSets) != 0 { + t.Fatal("pending hosted-zone change did not block SOA and child RecordSet observation") + } + provider.changes[pending.ID].Status = route53v1alpha1.Route53ChangeStatusInSync + reconcile() + reconcile() + if len(provider.upserted) != 1 { + t.Fatal("converged SOA was rewritten") + } + if state := mustZoneUnitStatusData(t, ctx, k8sClient, unit.Namespace, unit.Name); state.PendingHostedZoneChange != nil { + t.Fatalf("completed pending change retained: %#v", state.PendingHostedZoneChange) + } + } + assertCondition(t, unit.Status.Zone.Conditions, string(dnsv1alpha1.ConditionProgrammed), metav1.ConditionTrue, "Programmed") + var observed struct { + Minimum int32 `json:"negativeCacheTTLSeconds"` + } + if err := json.Unmarshal(unit.Status.Zone.Provider.Data.Raw, &observed); err != nil || observed.Minimum != test.wantTTL { + t.Fatalf("observed MINIMUM = %d, want %d; error %v", observed.Minimum, test.wantTTL, err) + } + actual := provider.records[key] + if test.wantWrites == 0 { + if !reflect.DeepEqual(actual, original) { + t.Fatalf("SOA changed without an UPSERT: %#v", actual) + } + } else { + want := "ns-1.awsdns-00.com. awsdns-hostmaster.amazon.com. 1 7200 900 1209600 " + strconv.FormatInt(int64(test.wantTTL), 10) + if !reflect.DeepEqual(actual.Values, []string{want}) { + t.Fatalf("SOA value = %v, want %q", actual.Values, want) + } + // The complete RRset must match apart from its final SOA field. + actual.Values = original.Values + if !reflect.DeepEqual(actual, original) { + t.Fatalf("UPSERT changed other RRset fields: %#v", actual) + } + } + if len(provider.created) != 0 || len(provider.deleted) != 0 { + t.Fatal("SOA reconciliation changed the hosted zone lifecycle") + } + if test.wantWrites != 0 { + provider.records[key] = original + reconcile() + if len(provider.upserted) != 2 || provider.upserted[1].Values[0] != provider.upserted[0].Values[0] { + t.Fatal("later SOA drift did not converge") + } + } + }) + } +} + +func TestSOARoundTrip(t *testing.T) { + for _, value := range []string{ + defaultSOA, + "ns.example. hostmaster.example. 4294967295 0010 020 0030 0", + "ns.example. hostmaster.example. 42 10 20 30 2147483647", + } { + t.Run(value, func(t *testing.T) { + soa, err := parseSOA(value) + if err != nil { + t.Fatal(err) + } + if got := soa.withMinimum(soa.minimum); got != value { + t.Fatalf("round trip = %q, want %q", got, value) + } + before := strings.Fields(value) + after := strings.Fields(soa.withMinimum(60)) + if !reflect.DeepEqual(before[:6], after[:6]) || after[6] != "60" { + t.Fatalf("SOA replacement changed other fields: %v", after) + } + }) + } +} + +func TestSOAMalformed(t *testing.T) { + for _, value := range []string{ + "", + "ns.example. hostmaster.example. 1 7200 900 1209600", + defaultSOA + " extra", + "ns.example. hostmaster.example. 1 7200 900 1209600 not-a-number", + "ns.example. hostmaster.example. 1 7200 900 1209600 -1", + "ns.example. hostmaster.example. 1 7200 900 1209600 2147483648", + } { + t.Run(value, func(t *testing.T) { + if _, err := parseSOA(value); err == nil { + t.Fatal("malformed SOA accepted") + } + }) + } +} diff --git a/internal/go/providers/route53/controllers/zoneunit/zone.go b/internal/go/providers/route53/controllers/zoneunit/zone.go index 37e0567..2d9c5a4 100644 --- a/internal/go/providers/route53/controllers/zoneunit/zone.go +++ b/internal/go/providers/route53/controllers/zoneunit/zone.go @@ -161,6 +161,10 @@ func (r *ZoneReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl. return ctrl.Result{}, err } + if result, done, err := r.reconcileNegativeCacheTTL(ctx, provider, &zone, hostedZone, params); done || err != nil { + return result, err + } + result = ctrl.Result{} result, err = r.refreshHostedZoneStatus(ctx, provider, &zone, hostedZone) if err != nil || result.Requeue || result.RequeueAfter > 0 { @@ -682,6 +686,7 @@ func (r *ZoneReconciler) clearHostedZoneProviderStatus(ctx context.Context, zone statusData.HostedZoneID = "" statusData.ZoneType = "" statusData.CallerReference = "" + statusData.NegativeCacheTTLSeconds = 0 statusData.PendingHostedZoneChange = nil statusData.PendingRecordSetChange = nil raw, err := json.Marshal(statusData) diff --git a/internal/go/providers/route53/controllers/zoneunit/zone_status.go b/internal/go/providers/route53/controllers/zoneunit/zone_status.go index 0937a38..dc909cd 100644 --- a/internal/go/providers/route53/controllers/zoneunit/zone_status.go +++ b/internal/go/providers/route53/controllers/zoneunit/zone_status.go @@ -249,8 +249,9 @@ func (r *ZoneReconciler) patchRoute53ZoneStatus(ctx context.Context, zone *dnsv1 } return r.patchZoneStatus(ctx, zone, func(status *dnsv1alpha1.ZoneStatus) { publicRaw, _ := json.Marshal(route53v1alpha1.Route53ZoneStatusData{ - HostedZoneID: statusData.HostedZoneID, - ZoneType: statusData.ZoneType, + HostedZoneID: statusData.HostedZoneID, + ZoneType: statusData.ZoneType, + NegativeCacheTTLSeconds: statusData.NegativeCacheTTLSeconds, }) status.Provider = &dnsv1alpha1.ProviderStatus{ Data: runtime.RawExtension{Raw: publicRaw}, @@ -317,6 +318,9 @@ func route53ZoneClassParameters(zoneClass *dnsv1alpha1.ZoneClass) (*route53v1alp if err := json.Unmarshal(zoneClass.Spec.Parameters.Raw, ¶ms); err != nil { return nil, fmt.Errorf("parameters must match Route 53 ZoneClass schema: %w", err) } + if params.NegativeCacheTTLSeconds != nil && *params.NegativeCacheTTLSeconds < 0 { + return nil, errors.New("negativeCacheTTLSeconds must be between 0 and 2147483647") + } return ¶ms, nil } diff --git a/internal/go/providers/route53/controllers/zoneunit/zone_test_fixtures_test.go b/internal/go/providers/route53/controllers/zoneunit/zone_test_fixtures_test.go index ab3d361..5c0c091 100644 --- a/internal/go/providers/route53/controllers/zoneunit/zone_test_fixtures_test.go +++ b/internal/go/providers/route53/controllers/zoneunit/zone_test_fixtures_test.go @@ -541,16 +541,19 @@ type createHostedZoneRequest struct { callerReference string } type fakeProvider struct { - zones map[string]HostedZone - tags map[string]map[string]string - changes map[string]*route53v1alpha1.Route53Change - records map[string]RecordSetResource - created []createHostedZoneRequest - deleted []string - upserted []RecordSetResource - deletedRRs []RecordSetResource - listed []string - fetched []string + zones map[string]HostedZone + tags map[string]map[string]string + changes map[string]*route53v1alpha1.Route53Change + records map[string]RecordSetResource + created []createHostedZoneRequest + deleted []string + upserted []RecordSetResource + deletedRRs []RecordSetResource + listed []string + fetched []string + readRecords []string + listedRecordSets []string + getRecordSetErr error changeRecordSetsErr func(hostedZoneID string, changes []RecordSetChange) error listHostedZonesByName func(domainName string) ([]HostedZone, error) @@ -634,6 +637,10 @@ func (p *fakeProvider) TagHostedZone(_ context.Context, id string, tags map[stri return nil } func (p *fakeProvider) GetRecordSet(_ context.Context, hostedZoneID, recordName string, recordType dnsv1alpha1.RecordType) (*RecordSetResource, error) { + p.readRecords = append(p.readRecords, recordKey(hostedZoneID, recordName, recordType)) + if p.getRecordSetErr != nil { + return nil, p.getRecordSetErr + } record, ok := p.records[recordKey(hostedZoneID, recordName, recordType)] if !ok { return nil, nil @@ -642,6 +649,7 @@ func (p *fakeProvider) GetRecordSet(_ context.Context, hostedZoneID, recordName return &copied, nil } func (p *fakeProvider) ListRecordSets(_ context.Context, hostedZoneID string) ([]RecordSetResource, error) { + p.listedRecordSets = append(p.listedRecordSets, hostedZoneID) var records []RecordSetResource for _, record := range p.records { if normalizeHostedZoneID(record.HostedZoneID) != normalizeHostedZoneID(hostedZoneID) { diff --git a/pkg/go/api/route53/v1alpha1/route53zone_types.go b/pkg/go/api/route53/v1alpha1/route53zone_types.go index a30654d..9c231f2 100644 --- a/pkg/go/api/route53/v1alpha1/route53zone_types.go +++ b/pkg/go/api/route53/v1alpha1/route53zone_types.go @@ -19,6 +19,12 @@ type Route53ZoneStatusData struct { // ZoneType is the observed Route 53 hosted zone type. ZoneType ZoneType `json:"zoneType,omitempty"` + // NegativeCacheTTLSeconds is the last observed SOA MINIMUM when managed by the class. + // +kubebuilder:validation:Minimum=0 + // +kubebuilder:validation:Maximum=2147483647 + // +optional + NegativeCacheTTLSeconds int32 `json:"negativeCacheTTLSeconds,omitempty"` + // CallerReference is the value passed to CreateHostedZone. CallerReference string `json:"callerReference,omitempty"` diff --git a/pkg/go/api/route53/v1alpha1/route53zoneclass_types.go b/pkg/go/api/route53/v1alpha1/route53zoneclass_types.go index 4f78cf2..de1b9be 100644 --- a/pkg/go/api/route53/v1alpha1/route53zoneclass_types.go +++ b/pkg/go/api/route53/v1alpha1/route53zoneclass_types.go @@ -51,6 +51,13 @@ type Route53ZoneClassParameters struct { // SameNameZonePolicy controls whether multiple hosted zones may share a domain name. SameNameZonePolicy SameNameZonePolicy `json:"sameNameZonePolicy,omitempty"` + // NegativeCacheTTLSeconds sets the SOA MINIMUM (RFC 2308 negative caching TTL) + // of hosted zones created or adopted by this class. Absent leaves the SOA untouched. + // +kubebuilder:validation:Minimum=0 + // +kubebuilder:validation:Maximum=2147483647 + // +optional + NegativeCacheTTLSeconds *int32 `json:"negativeCacheTTLSeconds,omitempty"` + // Tags are applied to Route 53 hosted zones created by this class. Tags map[string]string `json:"tags,omitempty"` } diff --git a/pkg/go/api/route53/v1alpha1/scheme_test.go b/pkg/go/api/route53/v1alpha1/scheme_test.go index 5de2b11..e90762e 100644 --- a/pkg/go/api/route53/v1alpha1/scheme_test.go +++ b/pkg/go/api/route53/v1alpha1/scheme_test.go @@ -35,3 +35,16 @@ func TestAddToSchemeRegistersRoute53Types(t *testing.T) { } } } + +func TestZoneClassParametersDeepCopyNegativeCacheTTL(t *testing.T) { + minimum := int32(60) + parameters := &route53v1alpha1.Route53ZoneClassParameters{NegativeCacheTTLSeconds: &minimum} + copied := parameters.DeepCopy() + *copied.NegativeCacheTTLSeconds = 0 + if *parameters.NegativeCacheTTLSeconds != 60 { + t.Fatal("deep copy shares negativeCacheTTLSeconds storage") + } + if (&route53v1alpha1.Route53ZoneClassParameters{}).DeepCopy().NegativeCacheTTLSeconds != nil { + t.Fatal("deep copy defaulted an absent negativeCacheTTLSeconds") + } +} diff --git a/pkg/go/api/route53/v1alpha1/zz_generated.deepcopy.go b/pkg/go/api/route53/v1alpha1/zz_generated.deepcopy.go index 5a7a045..1ac49ed 100644 --- a/pkg/go/api/route53/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/go/api/route53/v1alpha1/zz_generated.deepcopy.go @@ -290,6 +290,11 @@ func (in *Route53RuntimeCredentials) DeepCopy() *Route53RuntimeCredentials { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Route53ZoneClassParameters) DeepCopyInto(out *Route53ZoneClassParameters) { *out = *in + if in.NegativeCacheTTLSeconds != nil { + in, out := &in.NegativeCacheTTLSeconds, &out.NegativeCacheTTLSeconds + *out = new(int32) + **out = **in + } if in.Tags != nil { in, out := &in.Tags, &out.Tags *out = make(map[string]string, len(*in))