Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion controller/execute.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ func buildProvider(
case "dnsimple":
p, err = dnsimple.NewDnsimpleProvider(domainFilter, zoneIDFilter, cfg.DryRun)
case "coredns", "skydns":
p, err = coredns.NewCoreDNSProvider(domainFilter, cfg.CoreDNSPrefix, cfg.DryRun)
p, err = coredns.NewCoreDNSProvider(domainFilter, cfg.CoreDNSPrefix, cfg.TXTOwnerID, cfg.CoreDNSStrictlyOwned, cfg.DryRun)
case "exoscale":
p, err = exoscale.NewExoscaleProvider(
cfg.ExoscaleAPIEnvironment,
Expand Down
1 change: 1 addition & 0 deletions docs/flags.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@
| `--cloudflare-region-key=""` | When using the Cloudflare provider, specify the default region for Regional Services. Any value other than an empty string will enable the Regional Services feature (optional) |
| `--cloudflare-record-comment=""` | When using the Cloudflare provider, specify the comment for the DNS records (default: '') |
| `--coredns-prefix="/skydns/"` | When using the CoreDNS provider, specify the prefix name |
| `--[no-]coredns-strictly-owned` | When using the CoreDNS provider, store and filter strictly by txt-owner-id using an extra field inside of the etcd service (default: false) |
| `--akamai-serviceconsumerdomain=""` | When using the Akamai provider, specify the base URL (required when --provider=akamai and edgerc-path not specified) |
| `--akamai-client-token=""` | When using the Akamai provider, specify the client token (required when --provider=akamai and edgerc-path not specified) |
| `--akamai-client-secret=""` | When using the Akamai provider, specify the client secret (required when --provider=akamai and edgerc-path not specified) |
Expand Down
9 changes: 9 additions & 0 deletions docs/tutorials/coredns.md
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,15 @@ dnstools# dig @10.100.4.143 nginx.example.org +short
dnstools#
```

## Multi cluster support options

The CoreDNS provider allows records from different CoreDNS providers to be separated in a single etcd
by activating the setting `--coredns-strictly-onwed` flag and set `txt-owner-id`.

It will prevent any override (update/create/delete) of records by a different owner and prevent loading of records by a different owner.

This features works directly without any change to CoreDNS. CoreDNS will ignore this field inside the etcd record.

## Specific service annotation options

### Groups
Expand Down
2 changes: 1 addition & 1 deletion endpoint/endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,7 @@ func TestIsOwnedBy(t *testing.T) {
Labels: tt.fields.Labels,
}
if got := e.IsOwnedBy(tt.args.ownerID); got != tt.want {
t.Errorf("Endpoint.IsOwnedBy() = %v, want %v", got, tt.want)
t.Errorf("Endpoint.isOwnedBy() = %v, want %v", got, tt.want)
}
})
}
Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/externaldns/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ type Config struct {
CloudflareRegionalServices bool
CloudflareRegionKey string
CoreDNSPrefix string
CoreDNSStrictlyOwned bool
AkamaiServiceConsumerDomain string
AkamaiClientToken string
AkamaiClientSecret string
Expand Down Expand Up @@ -266,6 +267,7 @@ var defaultConfig = &Config{
Compatibility: "",
ConnectorSourceServer: "localhost:8080",
CoreDNSPrefix: "/skydns/",
CoreDNSStrictlyOwned: false,
CRDSourceAPIVersion: "externaldns.k8s.io/v1alpha1",
CRDSourceKind: "DNSEndpoint",
DefaultTargets: []string{},
Expand Down Expand Up @@ -697,6 +699,7 @@ func bindFlags(b FlagBinder, cfg *Config) {
b.StringVar("cloudflare-record-comment", "When using the Cloudflare provider, specify the comment for the DNS records (default: '')", "", &cfg.CloudflareDNSRecordsComment)

b.StringVar("coredns-prefix", "When using the CoreDNS provider, specify the prefix name", defaultConfig.CoreDNSPrefix, &cfg.CoreDNSPrefix)
b.BoolVar("coredns-strictly-owned", "When using the CoreDNS provider, store and filter strictly by txt-owner-id using an extra field inside of the etcd service (default: false)", defaultConfig.CoreDNSStrictlyOwned, &cfg.CoreDNSStrictlyOwned)
b.StringVar("akamai-serviceconsumerdomain", "When using the Akamai provider, specify the base URL (required when --provider=akamai and edgerc-path not specified)", defaultConfig.AkamaiServiceConsumerDomain, &cfg.AkamaiServiceConsumerDomain)
b.StringVar("akamai-client-token", "When using the Akamai provider, specify the client token (required when --provider=akamai and edgerc-path not specified)", defaultConfig.AkamaiClientToken, &cfg.AkamaiClientToken)
b.StringVar("akamai-client-secret", "When using the Akamai provider, specify the client secret (required when --provider=akamai and edgerc-path not specified)", defaultConfig.AkamaiClientSecret, &cfg.AkamaiClientSecret)
Expand Down
75 changes: 69 additions & 6 deletions provider/coredns/coredns.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,15 @@ type Service struct {

// Etcd key where we found this service and ignored from json un-/marshaling
Key string `json:"-"`

// OwnedBy is used to prevent service to be added by different external-dns (only used by external-dns)
OwnedBy string `json:"ownedby,omitempty"`
}

type etcdClient struct {
client *etcdcv3.Client
client *etcdcv3.Client
ownerID string
strictlyOwned bool
}

var _ coreDNSClient = etcdClient{}
Expand All @@ -110,12 +115,25 @@ func (c etcdClient) GetServices(ctx context.Context, prefix string) ([]*Service,
if err := json.Unmarshal(n.Value, svc); err != nil {
return nil, fmt.Errorf("%s: %w", n.Key, err)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}
}
if c.strictlyOwned && svc.OwnedBy != c.ownerID {
continue
}

You could just do Early ownership check: skip records not owned by this instance when strictlyOwned.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so most likely lines

if c.strictlyOwned {
			b.OwnedBy = svc.OwnedBy
		}

and

		if c.strictlyOwned && b.OwnedBy != c.ownerID {
			continue
		}

no longer required

b := Service{Host: svc.Host, Port: svc.Port, Priority: svc.Priority, Weight: svc.Weight, Text: svc.Text, Key: string(n.Key)}
b := Service{
Host: svc.Host,
Port: svc.Port,
Priority: svc.Priority,
Weight: svc.Weight,
Text: svc.Text,
Key: string(n.Key),
}
if c.strictlyOwned {
b.OwnedBy = svc.OwnedBy
}
if _, ok := bx[b]; ok {
// skip the service if already added to service list.
// the same service might be found in multiple etcd nodes.
continue
}
if c.strictlyOwned && b.OwnedBy != c.ownerID {
continue
}
bx[b] = true

svc.Key = string(n.Key)
Expand All @@ -132,6 +150,16 @@ func (c etcdClient) SaveService(ctx context.Context, service *Service) error {
ctx, cancel := context.WithTimeout(ctx, etcdTimeout)
defer cancel()

if ownedBy, err := c.isOwnedBy(ctx, service.Key); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we even need this check? Is there is an endpoint to create, we could create it and attach an owner. If there is same key but different owner, why we would even care?

return err
} else if !ownedBy {
return fmt.Errorf("key %q is not owned by this service", service.Key)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we trowing an error? This should be just ignore and continue with next?

}

if c.strictlyOwned {
service.OwnedBy = c.ownerID
}

value, err := json.Marshal(&service)
if err != nil {
return err
Expand All @@ -148,10 +176,45 @@ func (c etcdClient) DeleteService(ctx context.Context, key string) error {
ctx, cancel := context.WithTimeout(ctx, etcdTimeout)
defer cancel()

if owned, err := c.isOwnedBy(ctx, key); err != nil {
return err
} else if !owned {
return fmt.Errorf("key %q is not owned by this service", key)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, why we throwing an error? Is this not just a skip?

}

_, err := c.client.Delete(ctx, key, etcdcv3.WithPrefix())
return err
}

func (c etcdClient) isOwnedBy(ctx context.Context, key string) (bool, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to review that

if !c.strictlyOwned {
return true, nil
}

r, err := c.client.Get(ctx, key)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
r, err := c.client.Get(ctx, key)
r, err := c.client.Get(ctx, key, etcdcv3.WithLimit(1))

switch {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not look like go code. Let's not have switch, especially when there is error handling

if err != nil {
		return false, fmt.Errorf("etcd get %q: %w", key, err)
	}
	if r == nil || len(r.Kvs) == 0 {
		// Key missing -> treat as owned (safe to create)
		return true, nil
	}

case err != nil:
return false, err
case r == nil:
return true, nil
case len(r.Kvs) > 1:
return false, fmt.Errorf("found multiple keys with the same key this service")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please try to rephrase, I don't really understand the error message. I can guess but you can make this better

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rephrased

case len(r.Kvs) == 0:
return true, nil
}
for _, n := range r.Kvs {
svc := new(Service)
if err := json.Unmarshal(n.Value, svc); err != nil {
return false, fmt.Errorf("%s: %w", n.Key, err)
Copy link
Contributor

@szuecs szuecs Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fmt.Errorf("failed to unmarshal %q: %w", n.Key, err)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unified and changed.

}

if svc.OwnedBy == c.ownerID {
return true, nil
}
}
return false, nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return false, nil
svc, err := c.unmarshalService(r.Kvs[0])
if err != nil {
return false, fmt.Errorf("failed to unmarshal value for key %q: %w", key, err)
}
return svc.OwnedBy == c.ownerID, nil

As we passing full key, not sure how is possible to return 2 results

}

// builds etcd client config depending on connection scheme and TLS parameters
func getETCDConfig() (*etcdcv3.Config, error) {
etcdURLsStr := os.Getenv("ETCD_URLS")
Expand Down Expand Up @@ -183,7 +246,7 @@ func getETCDConfig() (*etcdcv3.Config, error) {
}

// the newETCDClient is an etcd client constructor
func newETCDClient() (coreDNSClient, error) {
func newETCDClient(ownerID string, strictlyOwned bool) (coreDNSClient, error) {
cfg, err := getETCDConfig()
if err != nil {
return nil, err
Expand All @@ -192,12 +255,12 @@ func newETCDClient() (coreDNSClient, error) {
if err != nil {
return nil, err
}
return etcdClient{c}, nil
return etcdClient{c, ownerID, strictlyOwned}, nil
}

// NewCoreDNSProvider is a CoreDNS provider constructor
func NewCoreDNSProvider(domainFilter *endpoint.DomainFilter, prefix string, dryRun bool) (provider.Provider, error) {
client, err := newETCDClient()
func NewCoreDNSProvider(domainFilter *endpoint.DomainFilter, prefix, ownerID string, strictlyOwned, dryRun bool) (provider.Provider, error) {
client, err := newETCDClient(ownerID, strictlyOwned)
if err != nil {
return nil, err
}
Expand Down
Loading
Loading