From 40dea167e74fc5c379afe7ce950308a2f80870eb Mon Sep 17 00:00:00 2001 From: Valentin Flaux <38909103+vflaux@users.noreply.github.com> Date: Thu, 22 May 2025 13:16:20 +0200 Subject: [PATCH 1/5] chore: reduce config validation cyclop reduce cyclomatic complexity of validation.ValidateConfig function --- pkg/apis/externaldns/validation/validation.go | 121 ++++++++++------- .../externaldns/validation/validation_test.go | 124 ++++++++++++++++++ 2 files changed, 196 insertions(+), 49 deletions(-) diff --git a/pkg/apis/externaldns/validation/validation.go b/pkg/apis/externaldns/validation/validation.go index a5d06fa9b..df8edccc1 100644 --- a/pkg/apis/externaldns/validation/validation.go +++ b/pkg/apis/externaldns/validation/validation.go @@ -28,57 +28,13 @@ import ( // ValidateConfig performs validation on the Config object func ValidateConfig(cfg *externaldns.Config) error { // TODO: Should probably return field.ErrorList - if cfg.LogFormat != "text" && cfg.LogFormat != "json" { - return fmt.Errorf("unsupported log format: %s", cfg.LogFormat) - } - if len(cfg.Sources) == 0 { - return errors.New("no sources specified") - } - if cfg.Provider == "" { - return errors.New("no provider specified") + + if err := preValidateConfig(cfg); err != nil { + return err } - // Azure provider specific validations - if cfg.Provider == "azure" { - if cfg.AzureConfigFile == "" { - return errors.New("no Azure config file specified") - } - } - - // Akamai provider specific validations - if cfg.Provider == "akamai" { - if cfg.AkamaiServiceConsumerDomain == "" && cfg.AkamaiEdgercPath != "" { - return errors.New("no Akamai ServiceConsumerDomain specified") - } - if cfg.AkamaiClientToken == "" && cfg.AkamaiEdgercPath != "" { - return errors.New("no Akamai client token specified") - } - if cfg.AkamaiClientSecret == "" && cfg.AkamaiEdgercPath != "" { - return errors.New("no Akamai client secret specified") - } - if cfg.AkamaiAccessToken == "" && cfg.AkamaiEdgercPath != "" { - return errors.New("no Akamai access token specified") - } - } - - if cfg.Provider == "rfc2136" { - if cfg.RFC2136MinTTL < 0 { - return errors.New("TTL specified for rfc2136 is negative") - } - - if cfg.RFC2136Insecure && cfg.RFC2136GSSTSIG { - return errors.New("--rfc2136-insecure and --rfc2136-gss-tsig are mutually exclusive arguments") - } - - if cfg.RFC2136GSSTSIG { - if cfg.RFC2136KerberosPassword == "" || cfg.RFC2136KerberosUsername == "" || cfg.RFC2136KerberosRealm == "" { - return errors.New("--rfc2136-kerberos-realm, --rfc2136-kerberos-username, and --rfc2136-kerberos-password are required when specifying --rfc2136-gss-tsig option") - } - } - - if cfg.RFC2136BatchChangeSize < 1 { - return errors.New("batch size specified for rfc2136 cannot be less than 1") - } + if err := validateConfigForProvider(cfg); err != nil { + return err } if cfg.IgnoreHostnameAnnotation && cfg.FQDNTemplate == "" { @@ -95,3 +51,70 @@ func ValidateConfig(cfg *externaldns.Config) error { } return nil } + +func preValidateConfig(cfg *externaldns.Config) error { + if cfg.LogFormat != "text" && cfg.LogFormat != "json" { + return fmt.Errorf("unsupported log format: %s", cfg.LogFormat) + } + if len(cfg.Sources) == 0 { + return errors.New("no sources specified") + } + if cfg.Provider == "" { + return errors.New("no provider specified") + } + return nil +} + +func validateConfigForProvider(cfg *externaldns.Config) error { + switch cfg.Provider { + case "azure": + return validateConfigForAzure(cfg) + case "akamai": + return validateConfigForAkamai(cfg) + case "rfc2136": + return validateConfigForRfc2136(cfg) + default: + return nil + } +} + +func validateConfigForAzure(cfg *externaldns.Config) error { + if cfg.AzureConfigFile == "" { + return errors.New("no Azure config file specified") + } + return nil +} + +func validateConfigForAkamai(cfg *externaldns.Config) error { + if cfg.AkamaiServiceConsumerDomain == "" && cfg.AkamaiEdgercPath != "" { + return errors.New("no Akamai ServiceConsumerDomain specified") + } + if cfg.AkamaiClientToken == "" && cfg.AkamaiEdgercPath != "" { + return errors.New("no Akamai client token specified") + } + if cfg.AkamaiClientSecret == "" && cfg.AkamaiEdgercPath != "" { + return errors.New("no Akamai client secret specified") + } + if cfg.AkamaiAccessToken == "" && cfg.AkamaiEdgercPath != "" { + return errors.New("no Akamai access token specified") + } + return nil +} + +func validateConfigForRfc2136(cfg *externaldns.Config) error { + if cfg.RFC2136MinTTL < 0 { + return errors.New("TTL specified for rfc2136 is negative") + } + if cfg.RFC2136Insecure && cfg.RFC2136GSSTSIG { + return errors.New("--rfc2136-insecure and --rfc2136-gss-tsig are mutually exclusive arguments") + } + if cfg.RFC2136GSSTSIG { + if cfg.RFC2136KerberosPassword == "" || cfg.RFC2136KerberosUsername == "" || cfg.RFC2136KerberosRealm == "" { + return errors.New("--rfc2136-kerberos-realm, --rfc2136-kerberos-username, and --rfc2136-kerberos-password are required when specifying --rfc2136-gss-tsig option") + } + } + if cfg.RFC2136BatchChangeSize < 1 { + return errors.New("batch size specified for rfc2136 cannot be less than 1") + } + return nil +} diff --git a/pkg/apis/externaldns/validation/validation_test.go b/pkg/apis/externaldns/validation/validation_test.go index 72dd45460..480220e68 100644 --- a/pkg/apis/externaldns/validation/validation_test.go +++ b/pkg/apis/externaldns/validation/validation_test.go @@ -50,6 +50,28 @@ func TestValidateFlags(t *testing.T) { cfg = newValidConfig(t) cfg.Provider = "" require.Error(t, ValidateConfig(cfg)) + + cfg = newValidConfig(t) + cfg.IgnoreHostnameAnnotation = true + cfg.FQDNTemplate = "" + require.Error(t, ValidateConfig(cfg)) + + cfg = newValidConfig(t) + cfg.TXTPrefix = "foo" + cfg.TXTSuffix = "bar" + require.Error(t, ValidateConfig(cfg)) + + cfg = newValidConfig(t) + cfg.LabelFilter = "foo" + require.NoError(t, ValidateConfig(cfg)) + + cfg = newValidConfig(t) + cfg.LabelFilter = "foo=bar" + require.NoError(t, ValidateConfig(cfg)) + + cfg = newValidConfig(t) + cfg.LabelFilter = "#invalid-selector" + require.Error(t, ValidateConfig(cfg)) } func newValidConfig(t *testing.T) *externaldns.Config { @@ -227,3 +249,105 @@ func TestValidateGoodRfc2136GssTsigConfig(t *testing.T) { assert.NoError(t, err) } } + +func TestValidateBadAkamaiConfig(t *testing.T) { + invalidAkamaiConfigs := []*externaldns.Config{ + { + LogFormat: "json", + Sources: []string{"test-source"}, + Provider: "akamai", + AkamaiClientToken: "test-token", + AkamaiClientSecret: "test-secret", + AkamaiAccessToken: "test-access-token", + AkamaiEdgercPath: "/path/to/edgerc", + // Missing AkamaiServiceConsumerDomain + }, + { + LogFormat: "json", + Sources: []string{"test-source"}, + Provider: "akamai", + AkamaiServiceConsumerDomain: "test-domain", + AkamaiClientSecret: "test-secret", + AkamaiAccessToken: "test-access-token", + AkamaiEdgercPath: "/path/to/edgerc", + // Missing AkamaiClientToken + }, + { + LogFormat: "json", + Sources: []string{"test-source"}, + Provider: "akamai", + AkamaiServiceConsumerDomain: "test-domain", + AkamaiClientToken: "test-token", + AkamaiAccessToken: "test-access-token", + AkamaiEdgercPath: "/path/to/edgerc", + // Missing AkamaiClientSecret + }, + { + LogFormat: "json", + Sources: []string{"test-source"}, + Provider: "akamai", + AkamaiServiceConsumerDomain: "test-domain", + AkamaiClientToken: "test-token", + AkamaiClientSecret: "test-secret", + AkamaiEdgercPath: "/path/to/edgerc", + // Missing AkamaiAccessToken + }, + } + + for _, cfg := range invalidAkamaiConfigs { + err := ValidateConfig(cfg) + assert.Error(t, err) + } +} + +func TestValidateGoodAkamaiConfig(t *testing.T) { + validAkamaiConfigs := []*externaldns.Config{ + { + LogFormat: "json", + Sources: []string{"test-source"}, + Provider: "akamai", + AkamaiServiceConsumerDomain: "test-domain", + AkamaiClientToken: "test-token", + AkamaiClientSecret: "test-secret", + AkamaiAccessToken: "test-access-token", + AkamaiEdgercPath: "/path/to/edgerc", + }, + { + LogFormat: "json", + Sources: []string{"test-source"}, + Provider: "akamai", + // All Akamai fields can be empty if AkamaiEdgercPath is not specified + }, + } + + for _, cfg := range validAkamaiConfigs { + err := ValidateConfig(cfg) + assert.NoError(t, err) + } +} + +func TestValidateBadAzureConfig(t *testing.T) { + cfg := externaldns.NewConfig() + + cfg.LogFormat = "json" + cfg.Sources = []string{"test-source"} + cfg.Provider = "azure" + // AzureConfigFile is empty + + err := ValidateConfig(cfg) + + assert.Error(t, err) +} + +func TestValidateGoodAzureConfig(t *testing.T) { + cfg := externaldns.NewConfig() + + cfg.LogFormat = "json" + cfg.Sources = []string{"test-source"} + cfg.Provider = "azure" + cfg.AzureConfigFile = "/path/to/azure.json" + + err := ValidateConfig(cfg) + + assert.NoError(t, err) +} From e7db933249e518c4727159ea4c3347e7f7ec24ec Mon Sep 17 00:00:00 2001 From: upsaurav12 Date: Thu, 22 May 2025 23:37:00 +0530 Subject: [PATCH 2/5] test(provider/gandi): bumped to 100% coverage --- provider/gandi/gandi_test.go | 40 ++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/provider/gandi/gandi_test.go b/provider/gandi/gandi_test.go index e3b4cca19..dc50d1ab2 100644 --- a/provider/gandi/gandi_test.go +++ b/provider/gandi/gandi_test.go @@ -452,6 +452,46 @@ func TestGandiProvider_ApplyChangesWithUnknownDomainDoesNoUpdate(t *testing.T) { }) } +func TestGandiProvider_ApplyChangesConvertsApexDomain(t *testing.T) { + changes := &plan.Changes{} + mockedClient := &mockGandiClient{} + mockedProvider := &GandiProvider{ + DomainClient: mockedClient, + LiveDNSClient: mockedClient, + } + + // Add a change where DNSName equals the zone name (apex domain) + changes.Create = []*endpoint.Endpoint{ + { + DNSName: "example.com", // Matches the zone name + Targets: endpoint.Targets{"192.168.0.1"}, + RecordType: "A", + RecordTTL: 666, + }, + } + + err := mockedProvider.ApplyChanges(context.Background(), changes) + if err != nil { + t.Errorf("should not fail, %s", err) + } + + td.Cmp(t, mockedClient.Actions, []MockAction{ + { + Name: "ListDomains", + }, + { + Name: "CreateDomainRecord", + FQDN: "example.com", + Record: livedns.DomainRecord{ + RrsetType: endpoint.RecordTypeA, + RrsetName: "@", + RrsetValues: []string{"192.168.0.1"}, + RrsetTTL: 666, + }, + }, + }) +} + func TestGandiProvider_FailingCases(t *testing.T) { changes := &plan.Changes{} changes.Create = []*endpoint.Endpoint{{DNSName: "test2.example.com", Targets: endpoint.Targets{"192.168.0.1"}, RecordType: "A", RecordTTL: 666}} From 7c0881477a6a96bd4063280a18dbaad5180c9b5d Mon Sep 17 00:00:00 2001 From: vflaux <38909103+vflaux@users.noreply.github.com> Date: Fri, 23 May 2025 16:42:37 +0200 Subject: [PATCH 3/5] chore(cloudflare): move regional services logic to dedicated file (#5329) * cloudflare: move regional logic to dedicated file * cloudflare: actions as enum type --- provider/cloudflare/cloudflare.go | 208 +--------- provider/cloudflare/cloudflare_regional.go | 210 ++++++++++ .../cloudflare/cloudflare_regional_test.go | 373 ++++++++++++++++++ provider/cloudflare/cloudflare_test.go | 334 +--------------- 4 files changed, 602 insertions(+), 523 deletions(-) create mode 100644 provider/cloudflare/cloudflare_regional.go create mode 100644 provider/cloudflare/cloudflare_regional_test.go diff --git a/provider/cloudflare/cloudflare.go b/provider/cloudflare/cloudflare.go index 8980adc99..03898ec18 100644 --- a/provider/cloudflare/cloudflare.go +++ b/provider/cloudflare/cloudflare.go @@ -38,13 +38,15 @@ import ( "sigs.k8s.io/external-dns/source/annotations" ) +type changeAction int + const ( // cloudFlareCreate is a ChangeAction enum value - cloudFlareCreate = "CREATE" + cloudFlareCreate changeAction = iota // cloudFlareDelete is a ChangeAction enum value - cloudFlareDelete = "DELETE" + cloudFlareDelete // cloudFlareUpdate is a ChangeAction enum value - cloudFlareUpdate = "UPDATE" + cloudFlareUpdate // defaultTTL 1 = automatic defaultTTL = 1 @@ -53,6 +55,16 @@ const ( paidZoneMaxCommentLength = 500 ) +var changeActionNames = map[changeAction]string{ + cloudFlareCreate: "CREATE", + cloudFlareDelete: "DELETE", + cloudFlareUpdate: "UPDATE", +} + +func (action changeAction) String() string { + return changeActionNames[action] +} + // We have to use pointers to bools now, as the upstream cloudflare-go library requires them // see: https://github.com/cloudflare/cloudflare-go/pull/595 @@ -78,11 +90,6 @@ type CustomHostnameIndex struct { type CustomHostnamesMap map[CustomHostnameIndex]cloudflare.CustomHostname -type DataLocalizationRegionalHostnameChange struct { - Action string - cloudflare.RegionalHostname -} - var recordTypeProxyNotSupported = map[string]bool{ "LOC": true, "MX": true, @@ -103,12 +110,6 @@ var recordTypeCustomHostnameSupported = map[string]bool{ "CNAME": true, } -var recordTypeRegionalHostnameSupported = map[string]bool{ - "A": true, - "AAAA": true, - "CNAME": true, -} - // cloudFlareDNS is the subset of the CloudFlare API that we actually use. Add methods as required. Signatures must match exactly. type cloudFlareDNS interface { UserDetails(ctx context.Context) (cloudflare.User, error) @@ -157,20 +158,6 @@ func (z zoneService) UpdateDNSRecord(ctx context.Context, rc *cloudflare.Resourc return err } -func (z zoneService) CreateDataLocalizationRegionalHostname(ctx context.Context, rc *cloudflare.ResourceContainer, rp cloudflare.CreateDataLocalizationRegionalHostnameParams) error { - _, err := z.service.CreateDataLocalizationRegionalHostname(ctx, rc, rp) - return err -} - -func (z zoneService) UpdateDataLocalizationRegionalHostname(ctx context.Context, rc *cloudflare.ResourceContainer, rp cloudflare.UpdateDataLocalizationRegionalHostnameParams) error { - _, err := z.service.UpdateDataLocalizationRegionalHostname(ctx, rc, rp) - return err -} - -func (z zoneService) DeleteDataLocalizationRegionalHostname(ctx context.Context, rc *cloudflare.ResourceContainer, hostname string) error { - return z.service.DeleteDataLocalizationRegionalHostname(ctx, rc, hostname) -} - func (z zoneService) DeleteDNSRecord(ctx context.Context, rc *cloudflare.ResourceContainer, recordID string) error { return z.service.DeleteDNSRecord(ctx, rc, recordID) } @@ -250,7 +237,7 @@ type CloudFlareProvider struct { // cloudFlareChange differentiates between ChangActions type cloudFlareChange struct { - Action string + Action changeAction ResourceRecord cloudflare.DNSRecord RegionalHostname cloudflare.RegionalHostname CustomHostnames map[string]cloudflare.CustomHostname @@ -273,22 +260,6 @@ func updateDNSRecordParam(cfc cloudFlareChange) cloudflare.UpdateDNSRecordParams } } -// createDataLocalizationRegionalHostnameParams is a function that returns the appropriate RegionalHostname Param based on the cloudFlareChange passed in -func createDataLocalizationRegionalHostnameParams(rhc DataLocalizationRegionalHostnameChange) cloudflare.CreateDataLocalizationRegionalHostnameParams { - return cloudflare.CreateDataLocalizationRegionalHostnameParams{ - Hostname: rhc.Hostname, - RegionKey: rhc.RegionKey, - } -} - -// updateDataLocalizationRegionalHostnameParams is a function that returns the appropriate RegionalHostname Param based on the cloudFlareChange passed in -func updateDataLocalizationRegionalHostnameParams(rhc DataLocalizationRegionalHostnameChange) cloudflare.UpdateDataLocalizationRegionalHostnameParams { - return cloudflare.UpdateDataLocalizationRegionalHostnameParams{ - Hostname: rhc.Hostname, - RegionKey: rhc.RegionKey, - } -} - // getCreateDNSRecordParam is a function that returns the appropriate Record Param based on the cloudFlareChange passed in func getCreateDNSRecordParam(cfc cloudFlareChange) cloudflare.CreateDNSRecordParams { return cloudflare.CreateDNSRecordParams{ @@ -538,129 +509,6 @@ func (p *CloudFlareProvider) submitCustomHostnameChanges(ctx context.Context, zo return !failedChange } -// submitDataLocalizationRegionalHostnameChanges applies a set of data localization regional hostname changes, returns false if it fails -func (p *CloudFlareProvider) submitDataLocalizationRegionalHostnameChanges(ctx context.Context, changes []DataLocalizationRegionalHostnameChange, resourceContainer *cloudflare.ResourceContainer) bool { - failedChange := false - - for _, change := range changes { - logFields := log.Fields{ - "hostname": change.Hostname, - "region_key": change.RegionKey, - "action": change.Action, - "zone": resourceContainer.Identifier, - } - log.WithFields(logFields).Info("Changing regional hostname") - switch change.Action { - case cloudFlareCreate: - log.WithFields(logFields).Debug("Creating regional hostname") - if p.DryRun { - continue - } - regionalHostnameParam := createDataLocalizationRegionalHostnameParams(change) - err := p.Client.CreateDataLocalizationRegionalHostname(ctx, resourceContainer, regionalHostnameParam) - if err != nil { - var apiErr *cloudflare.Error - if errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusConflict { - log.WithFields(logFields).Debug("Regional hostname already exists, updating instead") - params := updateDataLocalizationRegionalHostnameParams(change) - err := p.Client.UpdateDataLocalizationRegionalHostname(ctx, resourceContainer, params) - if err != nil { - failedChange = true - log.WithFields(logFields).Errorf("failed to update regional hostname: %v", err) - } - continue - } - failedChange = true - log.WithFields(logFields).Errorf("failed to create regional hostname: %v", err) - } - case cloudFlareUpdate: - log.WithFields(logFields).Debug("Updating regional hostname") - if p.DryRun { - continue - } - regionalHostnameParam := updateDataLocalizationRegionalHostnameParams(change) - err := p.Client.UpdateDataLocalizationRegionalHostname(ctx, resourceContainer, regionalHostnameParam) - if err != nil { - var apiErr *cloudflare.Error - if errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusNotFound { - log.WithFields(logFields).Debug("Regional hostname not does not exists, creating instead") - params := createDataLocalizationRegionalHostnameParams(change) - err := p.Client.CreateDataLocalizationRegionalHostname(ctx, resourceContainer, params) - if err != nil { - failedChange = true - log.WithFields(logFields).Errorf("failed to create regional hostname: %v", err) - } - continue - } - failedChange = true - log.WithFields(logFields).Errorf("failed to update regional hostname: %v", err) - } - case cloudFlareDelete: - log.WithFields(logFields).Debug("Deleting regional hostname") - if p.DryRun { - continue - } - err := p.Client.DeleteDataLocalizationRegionalHostname(ctx, resourceContainer, change.Hostname) - if err != nil { - var apiErr *cloudflare.Error - if errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusNotFound { - log.WithFields(logFields).Debug("Regional hostname does not exists, nothing to do") - continue - } - failedChange = true - log.WithFields(logFields).Errorf("failed to delete regional hostname: %v", err) - } - } - } - - return !failedChange -} - -// dataLocalizationRegionalHostnamesChanges processes a slice of cloudFlare changes and consolidates them -// into a list of data localization regional hostname changes. -// returns nil if no changes are needed -func dataLocalizationRegionalHostnamesChanges(changes []*cloudFlareChange) ([]DataLocalizationRegionalHostnameChange, error) { - regionalHostnameChanges := make(map[string]DataLocalizationRegionalHostnameChange) - for _, change := range changes { - if change.RegionalHostname.Hostname == "" { - continue - } - if change.RegionalHostname.RegionKey == "" { - return nil, fmt.Errorf("region key is empty for regional hostname %q", change.RegionalHostname.Hostname) - } - regionalHostname, ok := regionalHostnameChanges[change.RegionalHostname.Hostname] - switch change.Action { - case cloudFlareCreate, cloudFlareUpdate: - if !ok { - regionalHostnameChanges[change.RegionalHostname.Hostname] = DataLocalizationRegionalHostnameChange{ - Action: change.Action, - RegionalHostname: change.RegionalHostname, - } - continue - } - if regionalHostname.RegionKey != change.RegionalHostname.RegionKey { - return nil, fmt.Errorf("conflicting region keys for regional hostname %q: %q and %q", change.RegionalHostname.Hostname, regionalHostname.RegionKey, change.RegionalHostname.RegionKey) - } - if (change.Action == cloudFlareUpdate && regionalHostname.Action != cloudFlareUpdate) || - regionalHostname.Action == cloudFlareDelete { - regionalHostnameChanges[change.RegionalHostname.Hostname] = DataLocalizationRegionalHostnameChange{ - Action: cloudFlareUpdate, - RegionalHostname: change.RegionalHostname, - } - } - case cloudFlareDelete: - if !ok { - regionalHostnameChanges[change.RegionalHostname.Hostname] = DataLocalizationRegionalHostnameChange{ - Action: cloudFlareDelete, - RegionalHostname: change.RegionalHostname, - } - continue - } - } - } - return slices.Collect(maps.Values(regionalHostnameChanges)), nil -} - // submitChanges takes a zone and a collection of Changes and sends them as a single transaction. func (p *CloudFlareProvider) submitChanges(ctx context.Context, changes []*cloudFlareChange) error { // return early if there is nothing to change @@ -844,7 +692,7 @@ func (p *CloudFlareProvider) newCustomHostname(customHostname string, origin str } } -func (p *CloudFlareProvider) newCloudFlareChange(action string, ep *endpoint.Endpoint, target string, current *endpoint.Endpoint) *cloudFlareChange { +func (p *CloudFlareProvider) newCloudFlareChange(action changeAction, ep *endpoint.Endpoint, target string, current *endpoint.Endpoint) *cloudFlareChange { ttl := defaultTTL proxied := shouldBeProxied(ep, p.proxiedByDefault) @@ -862,13 +710,6 @@ func (p *CloudFlareProvider) newCloudFlareChange(action string, ep *endpoint.End newCustomHostnames[v] = p.newCustomHostname(v, ep.DNSName) } } - regionalHostname := cloudflare.RegionalHostname{} - if regionKey := getRegionKey(ep, p.RegionKey); regionKey != "" { - regionalHostname = cloudflare.RegionalHostname{ - Hostname: ep.DNSName, - RegionKey: regionKey, - } - } // Load comment from program flag comment := p.DNSRecordsConfig.Comment @@ -893,7 +734,7 @@ func (p *CloudFlareProvider) newCloudFlareChange(action string, ep *endpoint.End Content: target, Comment: comment, }, - RegionalHostname: regionalHostname, + RegionalHostname: p.regionalHostname(ep), CustomHostnamesPrev: prevCustomHostnames, CustomHostnames: newCustomHostnames, } @@ -1001,19 +842,6 @@ func shouldBeProxied(ep *endpoint.Endpoint, proxiedByDefault bool) bool { return proxied } -func getRegionKey(endpoint *endpoint.Endpoint, defaultRegionKey string) string { - if !recordTypeRegionalHostnameSupported[endpoint.RecordType] { - return "" - } - - for _, v := range endpoint.ProviderSpecific { - if v.Name == annotations.CloudflareRegionKey { - return v.Value - } - } - return defaultRegionKey -} - func getEndpointCustomHostnames(ep *endpoint.Endpoint) []string { for _, v := range ep.ProviderSpecific { if v.Name == annotations.CloudflareCustomHostnameKey { diff --git a/provider/cloudflare/cloudflare_regional.go b/provider/cloudflare/cloudflare_regional.go new file mode 100644 index 000000000..f2b355952 --- /dev/null +++ b/provider/cloudflare/cloudflare_regional.go @@ -0,0 +1,210 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cloudflare + +import ( + "context" + "errors" + "fmt" + "maps" + "net/http" + "slices" + + "github.com/cloudflare/cloudflare-go" + log "github.com/sirupsen/logrus" + + "sigs.k8s.io/external-dns/endpoint" + "sigs.k8s.io/external-dns/source/annotations" +) + +var recordTypeRegionalHostnameSupported = map[string]bool{ + "A": true, + "AAAA": true, + "CNAME": true, +} + +type regionalHostnameChange struct { + action changeAction + cloudflare.RegionalHostname +} + +func (z zoneService) CreateDataLocalizationRegionalHostname(ctx context.Context, rc *cloudflare.ResourceContainer, rp cloudflare.CreateDataLocalizationRegionalHostnameParams) error { + _, err := z.service.CreateDataLocalizationRegionalHostname(ctx, rc, rp) + return err +} + +func (z zoneService) UpdateDataLocalizationRegionalHostname(ctx context.Context, rc *cloudflare.ResourceContainer, rp cloudflare.UpdateDataLocalizationRegionalHostnameParams) error { + _, err := z.service.UpdateDataLocalizationRegionalHostname(ctx, rc, rp) + return err +} + +func (z zoneService) DeleteDataLocalizationRegionalHostname(ctx context.Context, rc *cloudflare.ResourceContainer, hostname string) error { + return z.service.DeleteDataLocalizationRegionalHostname(ctx, rc, hostname) +} + +// createDataLocalizationRegionalHostnameParams is a function that returns the appropriate RegionalHostname Param based on the cloudFlareChange passed in +func createDataLocalizationRegionalHostnameParams(rhc regionalHostnameChange) cloudflare.CreateDataLocalizationRegionalHostnameParams { + return cloudflare.CreateDataLocalizationRegionalHostnameParams{ + Hostname: rhc.Hostname, + RegionKey: rhc.RegionKey, + } +} + +// updateDataLocalizationRegionalHostnameParams is a function that returns the appropriate RegionalHostname Param based on the cloudFlareChange passed in +func updateDataLocalizationRegionalHostnameParams(rhc regionalHostnameChange) cloudflare.UpdateDataLocalizationRegionalHostnameParams { + return cloudflare.UpdateDataLocalizationRegionalHostnameParams{ + Hostname: rhc.Hostname, + RegionKey: rhc.RegionKey, + } +} + +// submitDataLocalizationRegionalHostnameChanges applies a set of data localization regional hostname changes, returns false if it fails +func (p *CloudFlareProvider) submitDataLocalizationRegionalHostnameChanges(ctx context.Context, rhChanges []regionalHostnameChange, resourceContainer *cloudflare.ResourceContainer) bool { + failedChange := false + + for _, rhChange := range rhChanges { + logFields := log.Fields{ + "hostname": rhChange.Hostname, + "region_key": rhChange.RegionKey, + "action": rhChange.action, + "zone": resourceContainer.Identifier, + } + log.WithFields(logFields).Info("Changing regional hostname") + switch rhChange.action { + case cloudFlareCreate: + log.WithFields(logFields).Debug("Creating regional hostname") + if p.DryRun { + continue + } + regionalHostnameParam := createDataLocalizationRegionalHostnameParams(rhChange) + err := p.Client.CreateDataLocalizationRegionalHostname(ctx, resourceContainer, regionalHostnameParam) + if err != nil { + var apiErr *cloudflare.Error + if errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusConflict { + log.WithFields(logFields).Debug("Regional hostname already exists, updating instead") + params := updateDataLocalizationRegionalHostnameParams(rhChange) + err := p.Client.UpdateDataLocalizationRegionalHostname(ctx, resourceContainer, params) + if err != nil { + failedChange = true + log.WithFields(logFields).Errorf("failed to update regional hostname: %v", err) + } + continue + } + failedChange = true + log.WithFields(logFields).Errorf("failed to create regional hostname: %v", err) + } + case cloudFlareUpdate: + log.WithFields(logFields).Debug("Updating regional hostname") + if p.DryRun { + continue + } + regionalHostnameParam := updateDataLocalizationRegionalHostnameParams(rhChange) + err := p.Client.UpdateDataLocalizationRegionalHostname(ctx, resourceContainer, regionalHostnameParam) + if err != nil { + var apiErr *cloudflare.Error + if errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusNotFound { + log.WithFields(logFields).Debug("Regional hostname not does not exists, creating instead") + params := createDataLocalizationRegionalHostnameParams(rhChange) + err := p.Client.CreateDataLocalizationRegionalHostname(ctx, resourceContainer, params) + if err != nil { + failedChange = true + log.WithFields(logFields).Errorf("failed to create regional hostname: %v", err) + } + continue + } + failedChange = true + log.WithFields(logFields).Errorf("failed to update regional hostname: %v", err) + } + case cloudFlareDelete: + log.WithFields(logFields).Debug("Deleting regional hostname") + if p.DryRun { + continue + } + err := p.Client.DeleteDataLocalizationRegionalHostname(ctx, resourceContainer, rhChange.Hostname) + if err != nil { + var apiErr *cloudflare.Error + if errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusNotFound { + log.WithFields(logFields).Debug("Regional hostname does not exists, nothing to do") + continue + } + failedChange = true + log.WithFields(logFields).Errorf("failed to delete regional hostname: %v", err) + } + } + } + + return !failedChange +} + +func (p *CloudFlareProvider) regionalHostname(ep *endpoint.Endpoint) cloudflare.RegionalHostname { + if p.RegionKey == "" || !recordTypeRegionalHostnameSupported[ep.RecordType] { + return cloudflare.RegionalHostname{} + } + regionKey := p.RegionKey + if epRegionKey, exists := ep.GetProviderSpecificProperty(annotations.CloudflareRegionKey); exists { + regionKey = epRegionKey + } + return cloudflare.RegionalHostname{ + Hostname: ep.DNSName, + RegionKey: regionKey, + } +} + +// dataLocalizationRegionalHostnamesChanges processes a slice of cloudFlare changes and consolidates them +// into a list of data localization regional hostname changes. +// returns nil if no changes are needed +func dataLocalizationRegionalHostnamesChanges(changes []*cloudFlareChange) ([]regionalHostnameChange, error) { + regionalHostnameChanges := make(map[string]regionalHostnameChange) + for _, change := range changes { + if change.RegionalHostname.Hostname == "" { + continue + } + if change.RegionalHostname.RegionKey == "" { + return nil, fmt.Errorf("region key is empty for regional hostname %q", change.RegionalHostname.Hostname) + } + regionalHostname, ok := regionalHostnameChanges[change.RegionalHostname.Hostname] + switch change.Action { + case cloudFlareCreate, cloudFlareUpdate: + if !ok { + regionalHostnameChanges[change.RegionalHostname.Hostname] = regionalHostnameChange{ + action: change.Action, + RegionalHostname: change.RegionalHostname, + } + continue + } + if regionalHostname.RegionKey != change.RegionalHostname.RegionKey { + return nil, fmt.Errorf("conflicting region keys for regional hostname %q: %q and %q", change.RegionalHostname.Hostname, regionalHostname.RegionKey, change.RegionalHostname.RegionKey) + } + if (change.Action == cloudFlareUpdate && regionalHostname.action != cloudFlareUpdate) || + regionalHostname.action == cloudFlareDelete { + regionalHostnameChanges[change.RegionalHostname.Hostname] = regionalHostnameChange{ + action: cloudFlareUpdate, + RegionalHostname: change.RegionalHostname, + } + } + case cloudFlareDelete: + if !ok { + regionalHostnameChanges[change.RegionalHostname.Hostname] = regionalHostnameChange{ + action: cloudFlareDelete, + RegionalHostname: change.RegionalHostname, + } + continue + } + } + } + return slices.Collect(maps.Values(regionalHostnameChanges)), nil +} diff --git a/provider/cloudflare/cloudflare_regional_test.go b/provider/cloudflare/cloudflare_regional_test.go new file mode 100644 index 000000000..f8273a601 --- /dev/null +++ b/provider/cloudflare/cloudflare_regional_test.go @@ -0,0 +1,373 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cloudflare + +import ( + "reflect" + "slices" + "testing" + + "github.com/cloudflare/cloudflare-go" + + "github.com/stretchr/testify/assert" + "sigs.k8s.io/external-dns/endpoint" +) + +func Test_regionalHostname(t *testing.T) { + type args struct { + endpoint *endpoint.Endpoint + defaultRegionKey string + } + tests := []struct { + name string + args args + want cloudflare.RegionalHostname + }{ + { + name: "no region key", + args: args{ + endpoint: &endpoint.Endpoint{ + RecordType: "A", + DNSName: "example.com", + }, + defaultRegionKey: "", + }, + want: cloudflare.RegionalHostname{}, + }, + { + name: "default region key", + args: args{ + endpoint: &endpoint.Endpoint{ + RecordType: "A", + DNSName: "example.com", + }, + defaultRegionKey: "us", + }, + want: cloudflare.RegionalHostname{ + Hostname: "example.com", + RegionKey: "us", + }, + }, + { + name: "endpoint with region key", + args: args{ + endpoint: &endpoint.Endpoint{ + RecordType: "A", + DNSName: "example.com", + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", + Value: "eu", + }, + }, + }, + defaultRegionKey: "us", + }, + want: cloudflare.RegionalHostname{ + Hostname: "example.com", + RegionKey: "eu", + }, + }, + { + name: "endpoint with empty region key", + args: args{ + endpoint: &endpoint.Endpoint{ + RecordType: "A", + DNSName: "example.com", + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", + Value: "", + }, + }, + }, + defaultRegionKey: "us", + }, + want: cloudflare.RegionalHostname{ + Hostname: "example.com", + RegionKey: "", + }, + }, + { + name: "unsupported record type", + args: args{ + endpoint: &endpoint.Endpoint{ + RecordType: "TXT", + DNSName: "example.com", + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", + Value: "eu", + }, + }, + }, + defaultRegionKey: "us", + }, + want: cloudflare.RegionalHostname{}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := CloudFlareProvider{RegionKey: tt.args.defaultRegionKey} + got := p.regionalHostname(tt.args.endpoint) + assert.Equal(t, got, tt.want) + }) + } +} + +func Test_dataLocalizationRegionalHostnamesChanges(t *testing.T) { + cmpDataLocalizationRegionalHostnameChange := func(i, j regionalHostnameChange) int { + if i.action == j.action { + return 0 + } + if i.Hostname < j.Hostname { + return -1 + } + return 1 + } + type args struct { + changes []*cloudFlareChange + } + tests := []struct { + name string + args args + want []regionalHostnameChange + wantErr bool + }{ + { + name: "empty input", + args: args{ + changes: []*cloudFlareChange{}, + }, + want: nil, + wantErr: false, + }, + { + name: "changes without RegionalHostname", + args: args{ + changes: []*cloudFlareChange{ + { + Action: cloudFlareCreate, + ResourceRecord: cloudflare.DNSRecord{ + Name: "example.com", + }, + RegionalHostname: cloudflare.RegionalHostname{}, // Empty + }, + }, + }, + want: nil, + wantErr: false, + }, + { + name: "change with empty RegionKey", + args: args{ + changes: []*cloudFlareChange{ + { + Action: cloudFlareCreate, + ResourceRecord: cloudflare.DNSRecord{ + Name: "example.com", + }, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "example.com", + RegionKey: "", // Empty region key + }, + }, + }, + }, + wantErr: true, + }, + { + name: "conflicting region keys", + args: args{ + changes: []*cloudFlareChange{ + { + Action: cloudFlareCreate, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "example.com", + RegionKey: "eu", + }, + }, + { + Action: cloudFlareCreate, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "example.com", + RegionKey: "us", // Different region key for same hostname + }, + }, + }, + }, + wantErr: true, + }, + { + name: "update takes precedence over create & delete", + args: args{ + changes: []*cloudFlareChange{ + { + Action: cloudFlareCreate, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "example.com", + RegionKey: "eu", + }, + }, + { + Action: cloudFlareUpdate, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "example.com", + RegionKey: "eu", + }, + }, + { + Action: cloudFlareDelete, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "example.com", + RegionKey: "eu", + }, + }, + }, + }, + want: []regionalHostnameChange{ + { + action: cloudFlareUpdate, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "example.com", + RegionKey: "eu", + }, + }, + }, + wantErr: false, + }, + { + name: "create after delete becomes update", + args: args{ + changes: []*cloudFlareChange{ + { + Action: cloudFlareDelete, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "example.com", + RegionKey: "eu", + }, + }, + { + Action: cloudFlareCreate, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "example.com", + RegionKey: "eu", + }, + }, + }, + }, + want: []regionalHostnameChange{ + { + action: cloudFlareUpdate, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "example.com", + RegionKey: "eu", + }, + }, + }, + wantErr: false, + }, + { + name: "consolidate mixed actions for different hostnames", + args: args{ + changes: []*cloudFlareChange{ + { + Action: cloudFlareCreate, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "example1.com", + RegionKey: "eu", + }, + }, + { + Action: cloudFlareUpdate, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "example2.com", + RegionKey: "us", + }, + }, + { + Action: cloudFlareDelete, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "example3.com", + RegionKey: "ap", + }, + }, + // duplicated actions + { + Action: cloudFlareCreate, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "example1.com", + RegionKey: "eu", + }, + }, + { + Action: cloudFlareUpdate, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "example2.com", + RegionKey: "us", + }, + }, + { + Action: cloudFlareDelete, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "example3.com", + RegionKey: "ap", + }, + }, + }, + }, + want: []regionalHostnameChange{ + { + action: cloudFlareCreate, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "example1.com", + RegionKey: "eu", + }, + }, + { + action: cloudFlareUpdate, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "example2.com", + RegionKey: "us", + }, + }, + { + action: cloudFlareDelete, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "example3.com", + RegionKey: "ap", + }, + }, + }, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := dataLocalizationRegionalHostnamesChanges(tt.args.changes) + if (err != nil) != tt.wantErr { + t.Errorf("dataLocalizationRegionalHostnamesChanges() error = %v, wantErr %v", err, tt.wantErr) + return + } + slices.SortFunc(got, cmpDataLocalizationRegionalHostnameChange) + slices.SortFunc(tt.want, cmpDataLocalizationRegionalHostnameChange) + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("dataLocalizationRegionalHostnamesChanges() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/provider/cloudflare/cloudflare_test.go b/provider/cloudflare/cloudflare_test.go index 893b315fe..97c20bb4d 100644 --- a/provider/cloudflare/cloudflare_test.go +++ b/provider/cloudflare/cloudflare_test.go @@ -21,13 +21,12 @@ import ( "errors" "fmt" "os" - "reflect" "slices" "sort" "strings" "testing" - cloudflare "github.com/cloudflare/cloudflare-go" + "github.com/cloudflare/cloudflare-go" "github.com/maxatome/go-testdeep/td" log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" @@ -2987,337 +2986,6 @@ func TestCloudflareListCustomHostnamesWithPagionation(t *testing.T) { assert.Len(t, chs, CustomHostnamesNumber) } -func Test_getRegionKey(t *testing.T) { - type args struct { - endpoint *endpoint.Endpoint - defaultRegionKey string - } - tests := []struct { - name string - args args - want string - }{ - { - name: "no region key", - args: args{ - endpoint: &endpoint.Endpoint{ - RecordType: "A", - }, - defaultRegionKey: "", - }, - want: "", - }, - { - name: "default region key", - args: args{ - endpoint: &endpoint.Endpoint{ - RecordType: "A", - }, - defaultRegionKey: "us", - }, - want: "us", - }, - { - name: "endpoint with region key", - args: args{ - endpoint: &endpoint.Endpoint{ - RecordType: "A", - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", - Value: "eu", - }, - }, - }, - defaultRegionKey: "us", - }, - want: "eu", - }, - { - name: "endpoint with empty region key", - args: args{ - endpoint: &endpoint.Endpoint{ - RecordType: "A", - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", - Value: "", - }, - }, - }, - defaultRegionKey: "us", - }, - want: "", - }, - { - name: "unsupported record type", - args: args{ - endpoint: &endpoint.Endpoint{ - RecordType: "TXT", - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", - Value: "eu", - }, - }, - }, - defaultRegionKey: "us", - }, - want: "", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := getRegionKey(tt.args.endpoint, tt.args.defaultRegionKey); got != tt.want { - t.Errorf("getRegionKey() = %v, want %v", got, tt.want) - } - }) - } -} - -func Test_dataLocalizationRegionalHostnamesChanges(t *testing.T) { - cmpDataLocalizationRegionalHostnameChange := func(i, j DataLocalizationRegionalHostnameChange) int { - if i.Action == j.Action { - return 0 - } - if i.Hostname < j.Hostname { - return -1 - } - return 1 - } - type args struct { - changes []*cloudFlareChange - } - tests := []struct { - name string - args args - want []DataLocalizationRegionalHostnameChange - wantErr bool - }{ - { - name: "empty input", - args: args{ - changes: []*cloudFlareChange{}, - }, - want: nil, - wantErr: false, - }, - { - name: "changes without RegionalHostname", - args: args{ - changes: []*cloudFlareChange{ - { - Action: cloudFlareCreate, - ResourceRecord: cloudflare.DNSRecord{ - Name: "example.com", - }, - RegionalHostname: cloudflare.RegionalHostname{}, // Empty - }, - }, - }, - want: nil, - wantErr: false, - }, - { - name: "change with empty RegionKey", - args: args{ - changes: []*cloudFlareChange{ - { - Action: cloudFlareCreate, - ResourceRecord: cloudflare.DNSRecord{ - Name: "example.com", - }, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example.com", - RegionKey: "", // Empty region key - }, - }, - }, - }, - wantErr: true, - }, - { - name: "conflicting region keys", - args: args{ - changes: []*cloudFlareChange{ - { - Action: cloudFlareCreate, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example.com", - RegionKey: "eu", - }, - }, - { - Action: cloudFlareCreate, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example.com", - RegionKey: "us", // Different region key for same hostname - }, - }, - }, - }, - wantErr: true, - }, - { - name: "update takes precedence over create & delete", - args: args{ - changes: []*cloudFlareChange{ - { - Action: cloudFlareCreate, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example.com", - RegionKey: "eu", - }, - }, - { - Action: cloudFlareUpdate, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example.com", - RegionKey: "eu", - }, - }, - { - Action: cloudFlareDelete, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example.com", - RegionKey: "eu", - }, - }, - }, - }, - want: []DataLocalizationRegionalHostnameChange{ - { - Action: cloudFlareUpdate, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example.com", - RegionKey: "eu", - }, - }, - }, - wantErr: false, - }, - { - name: "create after delete becomes update", - args: args{ - changes: []*cloudFlareChange{ - { - Action: cloudFlareDelete, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example.com", - RegionKey: "eu", - }, - }, - { - Action: cloudFlareCreate, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example.com", - RegionKey: "eu", - }, - }, - }, - }, - want: []DataLocalizationRegionalHostnameChange{ - { - Action: cloudFlareUpdate, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example.com", - RegionKey: "eu", - }, - }, - }, - wantErr: false, - }, - { - name: "consolidate mixed actions for different hostnames", - args: args{ - changes: []*cloudFlareChange{ - { - Action: cloudFlareCreate, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example1.com", - RegionKey: "eu", - }, - }, - { - Action: cloudFlareUpdate, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example2.com", - RegionKey: "us", - }, - }, - { - Action: cloudFlareDelete, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example3.com", - RegionKey: "ap", - }, - }, - // duplicated actions - { - Action: cloudFlareCreate, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example1.com", - RegionKey: "eu", - }, - }, - { - Action: cloudFlareUpdate, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example2.com", - RegionKey: "us", - }, - }, - { - Action: cloudFlareDelete, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example3.com", - RegionKey: "ap", - }, - }, - }, - }, - want: []DataLocalizationRegionalHostnameChange{ - { - Action: cloudFlareCreate, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example1.com", - RegionKey: "eu", - }, - }, - { - Action: cloudFlareUpdate, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example2.com", - RegionKey: "us", - }, - }, - { - Action: cloudFlareDelete, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example3.com", - RegionKey: "ap", - }, - }, - }, - wantErr: false, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := dataLocalizationRegionalHostnamesChanges(tt.args.changes) - if (err != nil) != tt.wantErr { - t.Errorf("dataLocalizationRegionalHostnamesChanges() error = %v, wantErr %v", err, tt.wantErr) - return - } - slices.SortFunc(got, cmpDataLocalizationRegionalHostnameChange) - slices.SortFunc(tt.want, cmpDataLocalizationRegionalHostnameChange) - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("dataLocalizationRegionalHostnamesChanges() = %v, want %v", got, tt.want) - } - }) - } -} - func TestZoneHasPaidPlan(t *testing.T) { client := NewMockCloudFlareClient() cfprovider := &CloudFlareProvider{ From d4013c22e272e4ffba901f70809ae5430d0ee04a Mon Sep 17 00:00:00 2001 From: Saurav Upadhyay <116784047+upsaurav12@users.noreply.github.com> Date: Fri, 23 May 2025 20:12:44 +0530 Subject: [PATCH 4/5] test(provider/civo): Improved test coverage to 90.4% (#5455) --- provider/civo/civo_test.go | 310 +++++++++++++++++++++++++++++++------ 1 file changed, 259 insertions(+), 51 deletions(-) diff --git a/provider/civo/civo_test.go b/provider/civo/civo_test.go index e476199f2..27020a274 100644 --- a/provider/civo/civo_test.go +++ b/provider/civo/civo_test.go @@ -129,6 +129,44 @@ func TestCivoProviderRecords(t *testing.T) { assert.Equal(t, int(records[1].RecordTTL), expected[1].TTL) } +func TestCivoProviderRecordsWithError(t *testing.T) { + client, server, _ := civogo.NewAdvancedClientForTesting([]civogo.ConfigAdvanceClientForTesting{ + { + Method: "GET", + Value: []civogo.ValueAdvanceClientForTesting{ + { + RequestBody: ``, + URL: "/v2/dns/12345/records", + ResponseBody: `[ + {"id": "1", "domain_id":"12345", "account_id": "1", "name": "", "type": "A", "value": "10.0.0.0", "ttl": 600}, + {"id": "2", "account_id": "1", "domain_id":"12345", "name": "", "type": "A", "value": "10.0.0.1", "ttl": 600} + ]`, + }, + { + RequestBody: ``, + URL: "/v2/dns", + ResponseBody: `invalid-json-data`, + }, + }, + }, + }) + + defer server.Close() + + provider := &CivoProvider{ + Client: *client, + domainFilter: endpoint.NewDomainFilter([]string{"example.com"}), + } + + _, err := client.ListDNSRecords("12345") + assert.NoError(t, err) + + endpoint, err := provider.Records(context.Background()) + assert.Error(t, err) + assert.Nil(t, endpoint) + +} + func TestCivoProviderWithoutRecords(t *testing.T) { client, server, _ := civogo.NewClientForTesting(map[string]string{ "/v2/dns/12345/records": `[]`, @@ -149,6 +187,68 @@ func TestCivoProviderWithoutRecords(t *testing.T) { assert.Empty(t, records) } +func TestCivoProcessCreateActionsLogs(t *testing.T) { + t.Run("Logs Skipping Zone, no creates found", func(t *testing.T) { + zonesByID := map[string]civogo.DNSDomain{ + "example.com": { + ID: "1", + AccountID: "1", + Name: "example.com", + }, + } + + recordsByZoneID := map[string][]civogo.DNSRecord{ + "example.com": { + { + ID: "1", + AccountID: "1", + Name: "abc", + Value: "12.12.12.1", + Type: "A", + TTL: 600, + }, + }, + } + + updateByZone := map[string][]*endpoint.Endpoint{ + "example.com": { + endpoint.NewEndpoint("abc.example.com", endpoint.RecordTypeA, "1.2.3.4"), + }, + } + var civoChanges CivoChanges + + err := processCreateActions(zonesByID, recordsByZoneID, updateByZone, &civoChanges) + require.NoError(t, err) + assert.Len(t, civoChanges.Creates, 1) + assert.Empty(t, civoChanges.Deletes) + assert.Empty(t, civoChanges.Updates) + }) + + t.Run("Records found which should not exist", func(t *testing.T) { + zonesByID := map[string]civogo.DNSDomain{ + "example.com": { + ID: "1", + AccountID: "1", + Name: "example.com", + }, + } + + recordsByZoneID := map[string][]civogo.DNSRecord{ + "example.com": {}, + } + + updateByZone := map[string][]*endpoint.Endpoint{ + "example.com": {}, + } + var civoChanges CivoChanges + + err := processCreateActions(zonesByID, recordsByZoneID, updateByZone, &civoChanges) + require.NoError(t, err) + assert.Empty(t, civoChanges.Creates) + assert.Empty(t, civoChanges.Creates) + assert.Empty(t, civoChanges.Updates) + }) +} func TestCivoProcessCreateActions(t *testing.T) { zoneByID := map[string]civogo.DNSDomain{ "example.com": { @@ -255,6 +355,41 @@ func TestCivoProcessCreateActionsWithError(t *testing.T) { assert.Equal(t, "invalid Record Type: AAAA", err.Error()) } +func TestCivoProcessUpdateActionsWithError(t *testing.T) { + zoneByID := map[string]civogo.DNSDomain{ + "example.com": { + ID: "1", + AccountID: "1", + Name: "example.com", + }, + } + + recordsByZoneID := map[string][]civogo.DNSRecord{ + "example.com": { + { + ID: "1", + AccountID: "1", + DNSDomainID: "1", + Name: "txt", + Value: "12.12.12.1", + Type: "A", + TTL: 600, + }, + }, + } + + updatesByZone := map[string][]*endpoint.Endpoint{ + "example.com": { + endpoint.NewEndpoint("foo.example.com", "AAAA", "1.2.3.4"), + endpoint.NewEndpoint("txt.example.com", endpoint.RecordTypeCNAME, "foo.example.com"), + }, + } + + var changes CivoChanges + err := processUpdateActions(zoneByID, recordsByZoneID, updatesByZone, &changes) + require.Error(t, err) +} + func TestCivoProcessUpdateActions(t *testing.T) { zoneByID := map[string]civogo.DNSDomain{ "example.com": { @@ -515,6 +650,64 @@ func TestCivoApplyChanges(t *testing.T) { assert.NoError(t, err) } +func TestCivoApplyChangesError(t *testing.T) { + client, server, _ := civogo.NewAdvancedClientForTesting([]civogo.ConfigAdvanceClientForTesting{ + { + Method: "GET", + Value: []civogo.ValueAdvanceClientForTesting{ + { + RequestBody: "", + URL: "/v2/dns", + ResponseBody: `[{"id": "12345", "account_id": "1", "name": "example.com"}]`, + }, + { + RequestBody: "", + URL: "/v2/dns/12345/records", + ResponseBody: `[]`, + }, + }, + }, + }) + + defer server.Close() + + provider := &CivoProvider{ + Client: *client, + } + + cases := []struct { + Name string + changes *plan.Changes + }{ + { + Name: "invalid record type from processCreateActions", + changes: &plan.Changes{ + Create: []*endpoint.Endpoint{ + endpoint.NewEndpoint("bad.example.com", "AAAA", "1.2.3.4"), + }, + }, + }, + { + Name: "invalid record type from processUpdateActions", + changes: &plan.Changes{ + UpdateOld: []*endpoint.Endpoint{ + endpoint.NewEndpoint("bad.example.com", "AAAA", "1.2.3.4"), + }, + UpdateNew: []*endpoint.Endpoint{ + endpoint.NewEndpoint("bad.example.com", "AAAA", "5.6.7.8"), + }, + }, + }, + } + + for _, tt := range cases { + t.Run(tt.Name, func(t *testing.T) { + err := provider.ApplyChanges(context.Background(), tt.changes) + assert.Equal(t, "invalid Record Type: AAAA", string(err.Error())) + }) + } +} + func TestCivoProviderFetchZones(t *testing.T) { client, server, _ := civogo.NewClientForTesting(map[string]string{ "/v2/dns": `[ @@ -688,39 +881,19 @@ func TestCivo_submitChangesCreate(t *testing.T) { }, }, }, - }) - defer server.Close() - - provider := &CivoProvider{ - Client: *client, - DryRun: false, - } - - changes := CivoChanges{ - Creates: []*CivoChangeCreate{ - { - Domain: civogo.DNSDomain{ - ID: "12345", - AccountID: "1", - Name: "example.com", + { + Method: "DELETE", + Value: []civogo.ValueAdvanceClientForTesting{ + { + URL: "/v2/dns/12345/records/76cc107f-fbef-4e2b-b97f-f5d34f4075d3", + ResponseBody: `{"result": "success"}`, }, - Options: &civogo.DNSRecordConfig{ - Type: "MX", - Name: "mail", - Value: "10.0.0.1", - Priority: 10, - TTL: 600, + { + URL: "/v2/dns/12345/records/error-record-id", + ResponseBody: `{"result": "error", "error": "failed to delete record"}`, }, }, }, - } - - err := provider.submitChanges(context.Background(), changes) - assert.NoError(t, err) -} - -func TestCivo_submitChangesUpdate(t *testing.T) { - client, server, _ := civogo.NewAdvancedClientForTesting([]civogo.ConfigAdvanceClientForTesting{ { Method: "PUT", Value: []civogo.ValueAdvanceClientForTesting{ @@ -738,6 +911,11 @@ func TestCivo_submitChangesUpdate(t *testing.T) { "ttl": 600 }`, }, + { + RequestBody: `{"type":"MX","name":"mail","value":"10.0.0.3","priority":10,"ttl":600}`, + URL: "/v2/dns/12345/records/error-record-id", + ResponseBody: `{"result": "error", "error": "failed to update record"}`, + }, }, }, }) @@ -745,36 +923,66 @@ func TestCivo_submitChangesUpdate(t *testing.T) { provider := &CivoProvider{ Client: *client, - DryRun: false, + DryRun: true, } - changes := CivoChanges{ - Updates: []*CivoChangeUpdate{ - { - Domain: civogo.DNSDomain{ID: "12345", AccountID: "1", Name: "example.com"}, - DomainRecord: civogo.DNSRecord{ - ID: "76cc107f-fbef-4e2b-b97f-f5d34f4075d3", - AccountID: "1", - DNSDomainID: "12345", - Name: "mail", - Value: "10.0.0.1", - Type: "MX", - Priority: 10, - TTL: 600, + cases := []struct { + name string + changes *CivoChanges + expectedResult error + }{ + { + name: "changes slice is empty", + changes: &CivoChanges{}, + expectedResult: nil, + }, + { + name: "changes slice has changes and update changes", + changes: &CivoChanges{ + Creates: []*CivoChangeCreate{ + { + Domain: civogo.DNSDomain{ + ID: "12345", + AccountID: "1", + Name: "example.com", + }, + Options: &civogo.DNSRecordConfig{ + Type: "MX", + Name: "mail", + Value: "10.0.0.1", + Priority: 10, + TTL: 600, + }, + }, }, - Options: civogo.DNSRecordConfig{ - Type: "MX", - Name: "mail", - Value: "10.0.0.2", - Priority: 10, - TTL: 600, + + Updates: []*CivoChangeUpdate{ + { + Domain: civogo.DNSDomain{ + ID: "12345", + AccountID: "2", + Name: "example.org", + }, + }, + { + Domain: civogo.DNSDomain{ + ID: "67890", + AccountID: "3", + Name: "example.COM", + }, + }, }, }, + expectedResult: nil, }, } - err := provider.submitChanges(context.Background(), changes) - assert.NoError(t, err) + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + err := provider.submitChanges(context.Background(), *c.changes) + assert.NoError(t, err) + }) + } } func TestCivo_submitChangesDelete(t *testing.T) { From d0e6a9075e3846ca134b137e0065aa94b62d2897 Mon Sep 17 00:00:00 2001 From: Michel Loiseleur <97035654+mloiseleur@users.noreply.github.com> Date: Fri, 23 May 2025 17:16:37 +0200 Subject: [PATCH 5/5] chore(crd): move code to `apis/v1alpha1` (#5446) * chore(crd): move code to `api/v1alpha1` * fix license check * fix linter * remove obsolete exclusion on linter --- .golangci.yml | 1 - Makefile | 3 +- apis/api.go | 17 ++++ apis/v1alpha1/api.go | 20 ++++ apis/v1alpha1/dnsendpoint.go | 62 ++++++++++++ apis/v1alpha1/groupversion_info.go | 40 ++++++++ apis/v1alpha1/zz_generated.deepcopy.go | 110 ++++++++++++++++++++++ charts/external-dns/crds/dnsendpoint.yaml | 3 + config/crd/standard/dnsendpoint.yaml | 3 + endpoint/endpoint.go | 46 +-------- endpoint/zz_generated.deepcopy.go | 89 ----------------- source/crd.go | 24 ++--- source/crd_test.go | 10 +- 13 files changed, 275 insertions(+), 153 deletions(-) create mode 100644 apis/api.go create mode 100644 apis/v1alpha1/api.go create mode 100644 apis/v1alpha1/dnsendpoint.go create mode 100644 apis/v1alpha1/groupversion_info.go create mode 100644 apis/v1alpha1/zz_generated.deepcopy.go diff --git a/.golangci.yml b/.golangci.yml index b7498ef48..e96def4ef 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -95,7 +95,6 @@ formatters: exclusions: generated: lax paths: - - endpoint/zz_generated.deepcopy.go - third_party$ - builtin$ - examples$ diff --git a/Makefile b/Makefile index c0c15cd2f..0ec5f712d 100644 --- a/Makefile +++ b/Makefile @@ -66,7 +66,8 @@ lint: licensecheck go-lint oas-lint #? crd: Generates CRD using controller-gen and copy it into chart .PHONY: crd crd: controller-gen-install - ${CONTROLLER_GEN} crd:crdVersions=v1 paths="./endpoint/..." output:crd:stdout > config/crd/standard/dnsendpoint.yaml + ${CONTROLLER_GEN} object crd:crdVersions=v1 paths="./endpoint/..." + ${CONTROLLER_GEN} object crd:crdVersions=v1 paths="./apis/..." output:crd:stdout > config/crd/standard/dnsendpoint.yaml cp -f config/crd/standard/dnsendpoint.yaml charts/external-dns/crds/dnsendpoint.yaml #? test: The verify target runs tasks similar to the CI tasks, but without code coverage diff --git a/apis/api.go b/apis/api.go new file mode 100644 index 000000000..a037bf3f3 --- /dev/null +++ b/apis/api.go @@ -0,0 +1,17 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package apis diff --git a/apis/v1alpha1/api.go b/apis/v1alpha1/api.go new file mode 100644 index 000000000..127a77368 --- /dev/null +++ b/apis/v1alpha1/api.go @@ -0,0 +1,20 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package v1alpha1 contains API Schema definitions for the externaldns.k8s.io v1alpha1 API group +// +kubebuilder:object:generate=true +// +groupName=externaldns.k8s.io +package v1alpha1 diff --git a/apis/v1alpha1/dnsendpoint.go b/apis/v1alpha1/dnsendpoint.go new file mode 100644 index 000000000..522bd1bee --- /dev/null +++ b/apis/v1alpha1/dnsendpoint.go @@ -0,0 +1,62 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha1 + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "sigs.k8s.io/external-dns/endpoint" +) + +// +genclient +// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object + +// DNSEndpoint is a contract that a user-specified CRD must implement to be used as a source for external-dns. +// The user-specified CRD should also have the status sub-resource. +// +k8s:openapi-gen=true +// +groupName=externaldns.k8s.io +// +kubebuilder:resource:path=dnsendpoints +// +kubebuilder:subresource:status +// +kubebuilder:metadata:annotations="api-approved.kubernetes.io=https://github.com/kubernetes-sigs/external-dns/pull/2007" +// +versionName=v1alpha1 +type DNSEndpoint struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec DNSEndpointSpec `json:"spec,omitempty"` + Status DNSEndpointStatus `json:"status,omitempty"` +} + +// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object +// DNSEndpointList is a list of DNSEndpoint objects +type DNSEndpointList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []DNSEndpoint `json:"items"` +} + +// DNSEndpointSpec defines the desired state of DNSEndpoint +type DNSEndpointSpec struct { + Endpoints []*endpoint.Endpoint `json:"endpoints,omitempty"` +} + +// DNSEndpointStatus defines the observed state of DNSEndpoint +type DNSEndpointStatus struct { + // The generation observed by the external-dns controller. + // +optional + ObservedGeneration int64 `json:"observedGeneration,omitempty"` +} diff --git a/apis/v1alpha1/groupversion_info.go b/apis/v1alpha1/groupversion_info.go new file mode 100644 index 000000000..926c4bc92 --- /dev/null +++ b/apis/v1alpha1/groupversion_info.go @@ -0,0 +1,40 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package v1alpha1 contains API Schema definitions for the externaldns.k8s.io v1alpha1 API group +// +kubebuilder:object:generate=true +// +groupName=externaldns.k8s.io +package v1alpha1 + +import ( + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/scheme" +) + +var ( + // GroupVersion is group version used to register these objects + GroupVersion = schema.GroupVersion{Group: "externaldns.k8s.io", Version: "v1alpha1"} + + // SchemeBuilder is used to add go types to the GroupVersionKind scheme + SchemeBuilder = &scheme.Builder{GroupVersion: GroupVersion} + + // AddToScheme adds the types in this group-version to the given scheme. + AddToScheme = SchemeBuilder.AddToScheme +) + +func init() { + SchemeBuilder.Register(&DNSEndpoint{}, &DNSEndpointList{}) +} diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go new file mode 100644 index 000000000..e2f5bf1b8 --- /dev/null +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -0,0 +1,110 @@ +//go:build !ignore_autogenerated + +// Code generated by controller-gen. DO NOT EDIT. + +package v1alpha1 + +import ( + runtime "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/external-dns/endpoint" +) + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DNSEndpoint) DeepCopyInto(out *DNSEndpoint) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + in.Spec.DeepCopyInto(&out.Spec) + out.Status = in.Status +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DNSEndpoint. +func (in *DNSEndpoint) DeepCopy() *DNSEndpoint { + if in == nil { + return nil + } + out := new(DNSEndpoint) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *DNSEndpoint) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DNSEndpointList) DeepCopyInto(out *DNSEndpointList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]DNSEndpoint, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DNSEndpointList. +func (in *DNSEndpointList) DeepCopy() *DNSEndpointList { + if in == nil { + return nil + } + out := new(DNSEndpointList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *DNSEndpointList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DNSEndpointSpec) DeepCopyInto(out *DNSEndpointSpec) { + *out = *in + if in.Endpoints != nil { + in, out := &in.Endpoints, &out.Endpoints + *out = make([]*endpoint.Endpoint, len(*in)) + for i := range *in { + if (*in)[i] != nil { + in, out := &(*in)[i], &(*out)[i] + *out = new(endpoint.Endpoint) + (*in).DeepCopyInto(*out) + } + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DNSEndpointSpec. +func (in *DNSEndpointSpec) DeepCopy() *DNSEndpointSpec { + if in == nil { + return nil + } + out := new(DNSEndpointSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DNSEndpointStatus) DeepCopyInto(out *DNSEndpointStatus) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DNSEndpointStatus. +func (in *DNSEndpointStatus) DeepCopy() *DNSEndpointStatus { + if in == nil { + return nil + } + out := new(DNSEndpointStatus) + in.DeepCopyInto(out) + return out +} diff --git a/charts/external-dns/crds/dnsendpoint.yaml b/charts/external-dns/crds/dnsendpoint.yaml index 88845aaae..83388d451 100644 --- a/charts/external-dns/crds/dnsendpoint.yaml +++ b/charts/external-dns/crds/dnsendpoint.yaml @@ -18,6 +18,9 @@ spec: - name: v1alpha1 schema: openAPIV3Schema: + description: |- + DNSEndpoint is a contract that a user-specified CRD must implement to be used as a source for external-dns. + The user-specified CRD should also have the status sub-resource. properties: apiVersion: description: |- diff --git a/config/crd/standard/dnsendpoint.yaml b/config/crd/standard/dnsendpoint.yaml index 88845aaae..83388d451 100644 --- a/config/crd/standard/dnsendpoint.yaml +++ b/config/crd/standard/dnsendpoint.yaml @@ -18,6 +18,9 @@ spec: - name: v1alpha1 schema: openAPIV3Schema: + description: |- + DNSEndpoint is a contract that a user-specified CRD must implement to be used as a source for external-dns. + The user-specified CRD should also have the status sub-resource. properties: apiVersion: description: |- diff --git a/endpoint/endpoint.go b/endpoint/endpoint.go index bbf67f641..86034fed6 100644 --- a/endpoint/endpoint.go +++ b/endpoint/endpoint.go @@ -24,8 +24,6 @@ import ( "strings" log "github.com/sirupsen/logrus" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) const ( @@ -337,48 +335,6 @@ func FilterEndpointsByOwnerID(ownerID string, eps []*Endpoint) []*Endpoint { return filtered } -// DNSEndpointSpec defines the desired state of DNSEndpoint -// +kubebuilder:object:generate=true -type DNSEndpointSpec struct { - Endpoints []*Endpoint `json:"endpoints,omitempty"` -} - -// DNSEndpointStatus defines the observed state of DNSEndpoint -type DNSEndpointStatus struct { - // The generation observed by the external-dns controller. - // +optional - ObservedGeneration int64 `json:"observedGeneration,omitempty"` -} - -// +genclient -// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object - -// DNSEndpoint is a contract that a user-specified CRD must implement to be used as a source for external-dns. -// The user-specified CRD should also have the status sub-resource. -// +k8s:openapi-gen=true -// +groupName=externaldns.k8s.io -// +kubebuilder:resource:path=dnsendpoints -// +kubebuilder:object:root=true -// +kubebuilder:subresource:status -// +kubebuilder:metadata:annotations="api-approved.kubernetes.io=https://github.com/kubernetes-sigs/external-dns/pull/2007" -// +versionName=v1alpha1 - -type DNSEndpoint struct { - metav1.TypeMeta `json:",inline"` - metav1.ObjectMeta `json:"metadata,omitempty"` - - Spec DNSEndpointSpec `json:"spec,omitempty"` - Status DNSEndpointStatus `json:"status,omitempty"` -} - -// +kubebuilder:object:root=true -// DNSEndpointList is a list of DNSEndpoint objects -type DNSEndpointList struct { - metav1.TypeMeta `json:",inline"` - metav1.ListMeta `json:"metadata,omitempty"` - Items []DNSEndpoint `json:"items"` -} - // RemoveDuplicates returns a slice holding the unique endpoints. // This function doesn't contemplate the Targets of an Endpoint // as part of the primary Key @@ -400,7 +356,7 @@ func RemoveDuplicates(endpoints []*Endpoint) []*Endpoint { return result } -// Check endpoint if is it properly formatted according to RFC standards +// CheckEndpoint Check if endpoint is properly formatted according to RFC standards func (e *Endpoint) CheckEndpoint() bool { switch recordType := e.RecordType; recordType { case RecordTypeMX: diff --git a/endpoint/zz_generated.deepcopy.go b/endpoint/zz_generated.deepcopy.go index ec07dace1..e86498d37 100644 --- a/endpoint/zz_generated.deepcopy.go +++ b/endpoint/zz_generated.deepcopy.go @@ -4,95 +4,6 @@ package endpoint -import ( - runtime "k8s.io/apimachinery/pkg/runtime" -) - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *DNSEndpoint) DeepCopyInto(out *DNSEndpoint) { - *out = *in - out.TypeMeta = in.TypeMeta - in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - in.Spec.DeepCopyInto(&out.Spec) - out.Status = in.Status -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DNSEndpoint. -func (in *DNSEndpoint) DeepCopy() *DNSEndpoint { - if in == nil { - return nil - } - out := new(DNSEndpoint) - in.DeepCopyInto(out) - return out -} - -// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. -func (in *DNSEndpoint) DeepCopyObject() runtime.Object { - if c := in.DeepCopy(); c != nil { - return c - } - return nil -} - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *DNSEndpointList) DeepCopyInto(out *DNSEndpointList) { - *out = *in - out.TypeMeta = in.TypeMeta - in.ListMeta.DeepCopyInto(&out.ListMeta) - if in.Items != nil { - in, out := &in.Items, &out.Items - *out = make([]DNSEndpoint, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DNSEndpointList. -func (in *DNSEndpointList) DeepCopy() *DNSEndpointList { - if in == nil { - return nil - } - out := new(DNSEndpointList) - in.DeepCopyInto(out) - return out -} - -// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. -func (in *DNSEndpointList) DeepCopyObject() runtime.Object { - if c := in.DeepCopy(); c != nil { - return c - } - return nil -} - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *DNSEndpointSpec) DeepCopyInto(out *DNSEndpointSpec) { - *out = *in - if in.Endpoints != nil { - in, out := &in.Endpoints, &out.Endpoints - *out = make([]*Endpoint, len(*in)) - for i := range *in { - if (*in)[i] != nil { - in, out := &(*in)[i], &(*out)[i] - *out = new(Endpoint) - (*in).DeepCopyInto(*out) - } - } - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DNSEndpointSpec. -func (in *DNSEndpointSpec) DeepCopy() *DNSEndpointSpec { - if in == nil { - return nil - } - out := new(DNSEndpointSpec) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Endpoint) DeepCopyInto(out *Endpoint) { *out = *in diff --git a/source/crd.go b/source/crd.go index d62805162..a603964dd 100644 --- a/source/crd.go +++ b/source/crd.go @@ -36,6 +36,7 @@ import ( "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" + apiv1alpha1 "sigs.k8s.io/external-dns/apis/v1alpha1" "sigs.k8s.io/external-dns/endpoint" ) @@ -53,8 +54,8 @@ type crdSource struct { func addKnownTypes(scheme *runtime.Scheme, groupVersion schema.GroupVersion) error { scheme.AddKnownTypes(groupVersion, - &endpoint.DNSEndpoint{}, - &endpoint.DNSEndpointList{}, + &apiv1alpha1.DNSEndpoint{}, + &apiv1alpha1.DNSEndpointList{}, ) metav1.AddToGroupVersion(scheme, groupVersion) return nil @@ -129,7 +130,7 @@ func NewCRDSource(crdClient rest.Interface, namespace, kind string, annotationFi return sourceCrd.watch(context.TODO(), &lo) }, }, - &endpoint.DNSEndpoint{}, + &apiv1alpha1.DNSEndpoint{}, 0) sourceCrd.informer = &informer go informer.Run(wait.NeverStop) @@ -164,7 +165,7 @@ func (cs *crdSource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, error endpoints := []*endpoint.Endpoint{} var ( - result *endpoint.DNSEndpointList + result *apiv1alpha1.DNSEndpointList err error ) @@ -174,7 +175,6 @@ func (cs *crdSource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, error } result, err = cs.filterByAnnotations(result) - if err != nil { return nil, err } @@ -229,7 +229,7 @@ func (cs *crdSource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, error return endpoints, nil } -func (cs *crdSource) setResourceLabel(crd *endpoint.DNSEndpoint, endpoints []*endpoint.Endpoint) { +func (cs *crdSource) setResourceLabel(crd *apiv1alpha1.DNSEndpoint, endpoints []*endpoint.Endpoint) { for _, ep := range endpoints { ep.Labels[endpoint.ResourceLabelKey] = fmt.Sprintf("crd/%s/%s", crd.Namespace, crd.Name) } @@ -244,8 +244,8 @@ func (cs *crdSource) watch(ctx context.Context, opts *metav1.ListOptions) (watch Watch(ctx) } -func (cs *crdSource) List(ctx context.Context, opts *metav1.ListOptions) (result *endpoint.DNSEndpointList, err error) { - result = &endpoint.DNSEndpointList{} +func (cs *crdSource) List(ctx context.Context, opts *metav1.ListOptions) (result *apiv1alpha1.DNSEndpointList, err error) { + result = &apiv1alpha1.DNSEndpointList{} err = cs.crdClient.Get(). Namespace(cs.namespace). Resource(cs.crdResource). @@ -255,8 +255,8 @@ func (cs *crdSource) List(ctx context.Context, opts *metav1.ListOptions) (result return } -func (cs *crdSource) UpdateStatus(ctx context.Context, dnsEndpoint *endpoint.DNSEndpoint) (result *endpoint.DNSEndpoint, err error) { - result = &endpoint.DNSEndpoint{} +func (cs *crdSource) UpdateStatus(ctx context.Context, dnsEndpoint *apiv1alpha1.DNSEndpoint) (result *apiv1alpha1.DNSEndpoint, err error) { + result = &apiv1alpha1.DNSEndpoint{} err = cs.crdClient.Put(). Namespace(dnsEndpoint.Namespace). Resource(cs.crdResource). @@ -269,7 +269,7 @@ func (cs *crdSource) UpdateStatus(ctx context.Context, dnsEndpoint *endpoint.DNS } // filterByAnnotations filters a list of dnsendpoints by a given annotation selector. -func (cs *crdSource) filterByAnnotations(dnsendpoints *endpoint.DNSEndpointList) (*endpoint.DNSEndpointList, error) { +func (cs *crdSource) filterByAnnotations(dnsendpoints *apiv1alpha1.DNSEndpointList) (*apiv1alpha1.DNSEndpointList, error) { labelSelector, err := metav1.ParseToLabelSelector(cs.annotationFilter) if err != nil { return nil, err @@ -284,7 +284,7 @@ func (cs *crdSource) filterByAnnotations(dnsendpoints *endpoint.DNSEndpointList) return dnsendpoints, nil } - filteredList := endpoint.DNSEndpointList{} + filteredList := apiv1alpha1.DNSEndpointList{} for _, dnsendpoint := range dnsendpoints.Items { // include dnsendpoint if its annotations match the selector diff --git a/source/crd_test.go b/source/crd_test.go index 53d8b322c..8386c6428 100644 --- a/source/crd_test.go +++ b/source/crd_test.go @@ -36,6 +36,7 @@ import ( "k8s.io/client-go/rest" "k8s.io/client-go/rest/fake" + apiv1alpha1 "sigs.k8s.io/external-dns/apis/v1alpha1" "sigs.k8s.io/external-dns/endpoint" ) @@ -61,8 +62,8 @@ func fakeRESTClient(endpoints []*endpoint.Endpoint, apiVersion, kind, namespace, scheme := runtime.NewScheme() addKnownTypes(scheme, groupVersion) - dnsEndpointList := endpoint.DNSEndpointList{} - dnsEndpoint := &endpoint.DNSEndpoint{ + dnsEndpointList := apiv1alpha1.DNSEndpointList{} + dnsEndpoint := &apiv1alpha1.DNSEndpoint{ TypeMeta: metav1.TypeMeta{ APIVersion: apiVersion, Kind: kind, @@ -74,7 +75,7 @@ func fakeRESTClient(endpoints []*endpoint.Endpoint, apiVersion, kind, namespace, Labels: labels, Generation: 1, }, - Spec: endpoint.DNSEndpointSpec{ + Spec: apiv1alpha1.DNSEndpointSpec{ Endpoints: endpoints, }, } @@ -101,7 +102,7 @@ func fakeRESTClient(endpoints []*endpoint.Endpoint, apiVersion, kind, namespace, case p == "/apis/"+apiVersion+"/namespaces/"+namespace+"/"+strings.ToLower(kind)+"s/"+name+"/status" && m == http.MethodPut: decoder := json.NewDecoder(req.Body) - var body endpoint.DNSEndpoint + var body apiv1alpha1.DNSEndpoint decoder.Decode(&body) dnsEndpoint.Status.ObservedGeneration = body.Status.ObservedGeneration return &http.Response{StatusCode: http.StatusOK, Header: defaultHeader(), Body: objBody(codec, dnsEndpoint)}, nil @@ -468,7 +469,6 @@ func testCRDSourceEndpoints(t *testing.T) { expectError: false, }, } { - t.Run(ti.title, func(t *testing.T) { t.Parallel()