diff --git a/controller/execute.go b/controller/execute.go index 7393a619d9..aecb1af56d 100644 --- a/controller/execute.go +++ b/controller/execute.go @@ -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, diff --git a/docs/flags.md b/docs/flags.md index f03026c99f..5e90795b65 100644 --- a/docs/flags.md +++ b/docs/flags.md @@ -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) | diff --git a/docs/tutorials/coredns.md b/docs/tutorials/coredns.md index 08bc38c30e..b885b98b50 100644 --- a/docs/tutorials/coredns.md +++ b/docs/tutorials/coredns.md @@ -261,6 +261,47 @@ 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-owned` 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. + +Flow: + +```mermaid +graph TD + subgraph ETCD + store--> E(services from Cluster A) + store--> F(services from Cluster B) + store--> G(services from someone else) + end + subgraph Cluster A + A(external-dns with stictly-owned) + end + A --> E + subgraph Cluster B + B(external-dns with stictly-owned) + end + B --> F + store --> CoreDNS +``` + +This features works directly without any change to CoreDNS. CoreDNS will ignore this field inside the etcd record. + +### Other entries inside etcd + +Service entries in etcd without an `ownedby` field will be filtered out by the provider if `strictly-owned` is activated. +Warning: If you activate `strictly-owned` afterwards, these entries will be ignored as the `ownedby` field is empty. + +### Ways to migrate to a multi cluster setup + +Ways: + +1. Add the correct owner to all services inside etcd by adding the field `ownedby` to the JSON. +2. Remove all services and allow them to be required again after restarting the provider. (Possible downtime.) + ## Specific service annotation options ### Groups diff --git a/endpoint/endpoint_test.go b/endpoint/endpoint_test.go index ef5fbc7821..0c40050762 100644 --- a/endpoint/endpoint_test.go +++ b/endpoint/endpoint_test.go @@ -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) } }) } diff --git a/pkg/apis/externaldns/types.go b/pkg/apis/externaldns/types.go index 5321b2636b..10653531a7 100644 --- a/pkg/apis/externaldns/types.go +++ b/pkg/apis/externaldns/types.go @@ -117,6 +117,7 @@ type Config struct { CloudflareRegionalServices bool CloudflareRegionKey string CoreDNSPrefix string + CoreDNSStrictlyOwned bool AkamaiServiceConsumerDomain string AkamaiClientToken string AkamaiClientSecret string @@ -266,6 +267,7 @@ var defaultConfig = &Config{ Compatibility: "", ConnectorSourceServer: "localhost:8080", CoreDNSPrefix: "/skydns/", + CoreDNSStrictlyOwned: false, CRDSourceAPIVersion: "externaldns.k8s.io/v1alpha1", CRDSourceKind: "DNSEndpoint", DefaultTargets: []string{}, @@ -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) diff --git a/provider/coredns/coredns.go b/provider/coredns/coredns.go index 6a93e35d62..861c01ba97 100644 --- a/provider/coredns/coredns.go +++ b/provider/coredns/coredns.go @@ -28,6 +28,7 @@ import ( "time" log "github.com/sirupsen/logrus" + "go.etcd.io/etcd/api/v3/mvccpb" etcdcv3 "go.etcd.io/etcd/client/v3" "sigs.k8s.io/external-dns/pkg/tlsutils" @@ -84,10 +85,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{} @@ -106,11 +112,21 @@ func (c etcdClient) GetServices(ctx context.Context, prefix string) ([]*Service, var svcs []*Service bx := make(map[Service]bool) for _, n := range r.Kvs { - svc := new(Service) - if err := json.Unmarshal(n.Value, svc); err != nil { - return nil, fmt.Errorf("%s: %w", n.Key, err) + svc, err := c.unmarshalService(n) + if err != nil { + return nil, err + } + if c.strictlyOwned && svc.OwnedBy != c.ownerID { + continue + } + 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 _, ok := bx[b]; ok { // skip the service if already added to service list. // the same service might be found in multiple etcd nodes. @@ -132,6 +148,25 @@ func (c etcdClient) SaveService(ctx context.Context, service *Service) error { ctx, cancel := context.WithTimeout(ctx, etcdTimeout) defer cancel() + // check only for empty OwnedBy + if c.strictlyOwned && service.OwnedBy != c.ownerID { + r, err := c.client.Get(ctx, service.Key) + if err != nil { + return fmt.Errorf("etcd get %q: %w", service.Key, err) + } + // Key missing -> treat as owned (safe to create) + if r != nil && len(r.Kvs) != 0 { + svc, err := c.unmarshalService(r.Kvs[0]) + if err != nil { + return fmt.Errorf("failed to unmarshal value for key %q: %w", service.Key, err) + } + if svc.OwnedBy != c.ownerID { + return fmt.Errorf("key %q is not owned by this provider", service.Key) + } + } + service.OwnedBy = c.ownerID + } + value, err := json.Marshal(&service) if err != nil { return err @@ -148,8 +183,38 @@ func (c etcdClient) DeleteService(ctx context.Context, key string) error { ctx, cancel := context.WithTimeout(ctx, etcdTimeout) defer cancel() - _, err := c.client.Delete(ctx, key, etcdcv3.WithPrefix()) - return err + if c.strictlyOwned { + rs, err := c.client.Get(ctx, key, etcdcv3.WithPrefix()) + if err != nil { + return err + } + for _, r := range rs.Kvs { + svc, err := c.unmarshalService(r) + if err != nil { + return err + } + if svc.OwnedBy != c.ownerID { + continue + } + + _, err = c.client.Delete(ctx, string(r.Key)) + if err != nil { + return err + } + } + return err + } else { + _, err := c.client.Delete(ctx, key, etcdcv3.WithPrefix()) + return err + } +} + +func (c etcdClient) unmarshalService(n *mvccpb.KeyValue) (*Service, error) { + svc := new(Service) + if err := json.Unmarshal(n.Value, svc); err != nil { + return nil, fmt.Errorf("failed to unmarshal %q: %w", n.Key, err) + } + return svc, nil } // builds etcd client config depending on connection scheme and TLS parameters @@ -183,7 +248,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 @@ -192,12 +257,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 } diff --git a/provider/coredns/coredns_test.go b/provider/coredns/coredns_test.go index e92b5f368a..5b8c930e1e 100644 --- a/provider/coredns/coredns_test.go +++ b/provider/coredns/coredns_test.go @@ -75,14 +75,24 @@ func (m *MockEtcdKV) Put(ctx context.Context, key, input string, _ ...etcdcv3.Op return args.Get(0).(*etcdcv3.PutResponse), args.Error(1) } -func (m *MockEtcdKV) Get(ctx context.Context, key string, _ ...etcdcv3.OpOption) (*etcdcv3.GetResponse, error) { - args := m.Called(ctx, key) - return args.Get(0).(*etcdcv3.GetResponse), args.Error(1) +func (m *MockEtcdKV) Get(ctx context.Context, key string, opts ...etcdcv3.OpOption) (*etcdcv3.GetResponse, error) { + if len(opts) == 0 { + args := m.Called(ctx, key) + return args.Get(0).(*etcdcv3.GetResponse), args.Error(1) + } else { + args := m.Called(ctx, key, opts[0]) + return args.Get(0).(*etcdcv3.GetResponse), args.Error(1) + } } func (m *MockEtcdKV) Delete(ctx context.Context, key string, opts ...etcdcv3.OpOption) (*etcdcv3.DeleteResponse, error) { - args := m.Called(ctx, key, opts[0]) - return args.Get(0).(*etcdcv3.DeleteResponse), args.Error(1) + if len(opts) == 0 { + args := m.Called(ctx, key) + return args.Get(0).(*etcdcv3.DeleteResponse), args.Error(1) + } else { + args := m.Called(ctx, key, opts[0]) + return args.Get(0).(*etcdcv3.DeleteResponse), args.Error(1) + } } func TestETCDConfig(t *testing.T) { @@ -494,14 +504,15 @@ func TestGetServices_Success(t *testing.T) { value, err := json.Marshal(svc) require.NoError(t, err) mockKV := new(MockEtcdKV) - mockKV.On("Get", mock.Anything, "/prefix").Return(&etcdcv3.GetResponse{ - Kvs: []*mvccpb.KeyValue{ - { - Key: []byte("/prefix/1"), - Value: value, + mockKV.On("Get", mock.Anything, "/prefix", mock.AnythingOfType("clientv3.OpOption")). + Return(&etcdcv3.GetResponse{ + Kvs: []*mvccpb.KeyValue{ + { + Key: []byte("/prefix/1"), + Value: value, + }, }, - }, - }, nil) + }, nil) c := etcdClient{ client: &etcdcv3.Client{ @@ -527,18 +538,19 @@ func TestGetServices_Duplicate(t *testing.T) { value, err := json.Marshal(svc) require.NoError(t, err) - mockKV.On("Get", mock.Anything, "/prefix").Return(&etcdcv3.GetResponse{ - Kvs: []*mvccpb.KeyValue{ - { - Key: []byte("/prefix/1"), - Value: value, - }, - { - Key: []byte("/prefix/1"), - Value: value, + mockKV.On("Get", mock.Anything, "/prefix", mock.AnythingOfType("clientv3.OpOption")). + Return(&etcdcv3.GetResponse{ + Kvs: []*mvccpb.KeyValue{ + { + Key: []byte("/prefix/1"), + Value: value, + }, + { + Key: []byte("/prefix/1"), + Value: value, + }, }, - }, - }, nil) + }, nil) result, err := c.GetServices(context.Background(), "/prefix") assert.NoError(t, err) @@ -560,18 +572,19 @@ func TestGetServices_Multiple(t *testing.T) { value2, err := json.Marshal(svc2) require.NoError(t, err) - mockKV.On("Get", mock.Anything, "/prefix").Return(&etcdcv3.GetResponse{ - Kvs: []*mvccpb.KeyValue{ - { - Key: []byte("/prefix/1"), - Value: value, - }, - { - Key: []byte("/prefix/2"), - Value: value2, + mockKV.On("Get", mock.Anything, "/prefix", mock.AnythingOfType("clientv3.OpOption")). + Return(&etcdcv3.GetResponse{ + Kvs: []*mvccpb.KeyValue{ + { + Key: []byte("/prefix/1"), + Value: value, + }, + { + Key: []byte("/prefix/2"), + Value: value2, + }, }, - }, - }, nil) + }, nil) result, err := c.GetServices(context.Background(), "/prefix") assert.NoError(t, err) @@ -579,26 +592,114 @@ func TestGetServices_Multiple(t *testing.T) { assert.Equal(t, priority, result[1].Priority) } -func TestGetServices_UnmarshalError(t *testing.T) { +func TestGetServices_FilterOutOtherServicesOwnerIDSetButNothingChanged(t *testing.T) { mockKV := new(MockEtcdKV) c := etcdClient{ client: &etcdcv3.Client{ KV: mockKV, }, + ownerID: "owner", + strictlyOwned: false, } - mockKV.On("Get", mock.Anything, "/prefix").Return(&etcdcv3.GetResponse{ - Kvs: []*mvccpb.KeyValue{ - { - Key: []byte("/prefix/1"), - Value: []byte("invalid-json"), + svc := Service{Host: "example.com", Port: 80, Priority: 1, Weight: 10, Text: "hello", OwnedBy: "owner"} + value, err := json.Marshal(svc) + require.NoError(t, err) + svc2 := Service{Host: "example.com", Port: 80, Priority: 0, Weight: 10, Text: "hello", OwnedBy: ""} + value2, err := json.Marshal(svc2) + require.NoError(t, err) + svc3 := Service{Host: "example.com", Port: 80, Priority: 0, Weight: 10, Text: "hello", OwnedBy: "managed-by-someone-else"} + value3, err := json.Marshal(svc3) + require.NoError(t, err) + + mockKV.On("Get", mock.Anything, "/prefix", mock.AnythingOfType("clientv3.OpOption")). + Return(&etcdcv3.GetResponse{ + Kvs: []*mvccpb.KeyValue{ + { + Key: []byte("/prefix/1"), + Value: value, + }, + { + Key: []byte("/prefix/2"), + Value: value2, + }, + { + Key: []byte("/prefix/3"), + Value: value3, + }, }, - { - Key: []byte("/prefix/1"), - Value: []byte("invalid-json"), + }, nil) + + result, err := c.GetServices(context.Background(), "/prefix") + assert.NoError(t, err) + assert.Len(t, result, 3) +} + +func TestGetServices_FilterOutOtherServicesWithStrictlyOwned(t *testing.T) { + mockKV := new(MockEtcdKV) + c := etcdClient{ + client: &etcdcv3.Client{ + KV: mockKV, + }, + ownerID: "owned-by", + strictlyOwned: true, + } + + svc := Service{Host: "example.com", Port: 80, Priority: 1, Weight: 10, Text: "hello", OwnedBy: "owned-by"} + value, err := json.Marshal(svc) + require.NoError(t, err) + svc2 := Service{Host: "example.com", Port: 80, Priority: 0, Weight: 10, Text: "hello", OwnedBy: ""} + value2, err := json.Marshal(svc2) + require.NoError(t, err) + svc3 := Service{Host: "example.com", Port: 80, Priority: 0, Weight: 10, Text: "hello", OwnedBy: "owned-by-someone-else"} + value3, err := json.Marshal(svc3) + require.NoError(t, err) + + mockKV.On("Get", mock.Anything, "/prefix", mock.AnythingOfType("clientv3.OpOption")). + Return(&etcdcv3.GetResponse{ + Kvs: []*mvccpb.KeyValue{ + { + Key: []byte("/prefix/1"), + Value: value, + }, + { + Key: []byte("/prefix/2"), + Value: value2, + }, + { + Key: []byte("/prefix/3"), + Value: value3, + }, }, + }, nil) + + result, err := c.GetServices(context.Background(), "/prefix") + assert.NoError(t, err) + assert.Len(t, result, 1) + assert.Equal(t, "owned-by", result[0].OwnedBy) +} + +func TestGetServices_UnmarshalError(t *testing.T) { + mockKV := new(MockEtcdKV) + c := etcdClient{ + client: &etcdcv3.Client{ + KV: mockKV, }, - }, nil) + } + + mockKV.On("Get", mock.Anything, "/prefix", mock.AnythingOfType("clientv3.OpOption")). + Return(&etcdcv3.GetResponse{ + Kvs: []*mvccpb.KeyValue{ + { + Key: []byte("/prefix/1"), + Value: []byte("invalid-json"), + }, + { + Key: []byte("/prefix/1"), + Value: []byte("invalid-json"), + }, + }, + }, nil) _, err := c.GetServices(context.Background(), "/prefix") assert.Error(t, err) @@ -613,7 +714,8 @@ func TestGetServices_GetError(t *testing.T) { }, } - mockKV.On("Get", mock.Anything, "/prefix").Return(&etcdcv3.GetResponse{}, errors.New("etcd failure")) + mockKV.On("Get", mock.Anything, "/prefix", mock.AnythingOfType("clientv3.OpOption")). + Return(&etcdcv3.GetResponse{}, errors.New("etcd failure")) _, err := c.GetServices(context.Background(), "/prefix") assert.Error(t, err) @@ -664,12 +766,151 @@ func TestDeleteService(t *testing.T) { } } +func TestDeleteServiceWithStrictlyOwned(t *testing.T) { + tests := []struct { + name string + ownerID string + key string + existingServices []Service + deletedKeys []string + }{ + { + name: "successful deletion with owned by (same) with strictly owned", + key: "/skydns/local/test", + ownerID: "owned-by", + existingServices: []Service{{ + Host: "example.com", + Port: 80, + Priority: 1, + Weight: 10, + Text: "hello", + Key: "/skydns/local/test", + OwnedBy: "owned-by", + }}, + deletedKeys: []string{"/skydns/local/test"}, + }, + { + name: "prevent deletion with owned by (no one) with strictly owned", + key: "/skydns/local/test", + ownerID: "owned-by", + existingServices: []Service{{ + Host: "example.com", + Port: 80, + Priority: 1, + Weight: 10, + Text: "hello", + Key: "/skydns/local/test", + }}, + deletedKeys: []string{}, + }, + { + name: "prevent deletion with owned by (other) with strictly owned", + key: "/skydns/local/test", + ownerID: "owned-by", + existingServices: []Service{{ + Host: "example.com", + Port: 80, + Priority: 1, + Weight: 10, + Text: "hello", + Key: "/skydns/local/test", + OwnedBy: "owned-by-other", + }}, + deletedKeys: []string{}, + }, + { + name: "successful partial deletion with owned by (same) with strictly owned", + key: "/skydns/local/test", + ownerID: "owned-by", + existingServices: []Service{ + { + Host: "example.com", + Port: 80, + Priority: 1, + Weight: 10, + Text: "hello", + Key: "/skydns/local/test/1", + OwnedBy: "owned-by", + }, + { + Host: "example.com", + Port: 80, + Priority: 1, + Weight: 10, + Text: "hello", + Key: "/skydns/local/test/2", + }, + { + Host: "example.com", + Port: 80, + Priority: 1, + Weight: 10, + Text: "hello", + Key: "/skydns/local/test/3", + OwnedBy: "owned-by-other", + }, + { + Host: "example.com", + Port: 80, + Priority: 1, + Weight: 10, + Text: "hello", + Key: "/skydns/local/test/4", + OwnedBy: "owned-by", + }, + }, + deletedKeys: []string{"/skydns/local/test/1", "/skydns/local/test/4"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockKV := new(MockEtcdKV) + for _, key := range tt.deletedKeys { + mockKV.On("Delete", mock.Anything, key). + Return(&etcdcv3.DeleteResponse{}, nil) + } + kvs := []*mvccpb.KeyValue{} + for _, service := range tt.existingServices { + actualValue, err := json.Marshal(&service) + require.NoError(t, err) + kvs = append(kvs, &mvccpb.KeyValue{ + Key: []byte(service.Key), + Value: actualValue, + }) + } + + mockKV.On("Get", mock.Anything, tt.key, mock.AnythingOfType("clientv3.OpOption")).Return(&etcdcv3.GetResponse{ + Kvs: kvs, + }, nil) + + c := etcdClient{ + client: &etcdcv3.Client{ + KV: mockKV, + }, + ownerID: tt.ownerID, + strictlyOwned: true, + } + + err := c.DeleteService(context.Background(), tt.key) + + require.NoError(t, err) + mockKV.AssertExpectations(t) + }) + } +} + func TestSaveService(t *testing.T) { type testCase struct { - name string - service *Service - mockPutErr error - wantErr bool + name string + ownerID string + strictlyOwned bool + service *Service + expectedService *Service + exists bool + ignoreGetCall bool + mockPutErr error + wantErr bool } tests := []testCase{ { @@ -682,6 +923,181 @@ func TestSaveService(t *testing.T) { Text: "hello", Key: "/prefix/1", }, + expectedService: &Service{ + Host: "example.com", + Port: 80, + Priority: 1, + Weight: 10, + Text: "hello", + Key: "/prefix/1", + }, + }, + { + name: "success with 'owned-by' without strictly owned", + ownerID: "owned-by", + exists: true, + service: &Service{ + Host: "example.com", + Port: 80, + Priority: 1, + Weight: 10, + Text: "hello", + Key: "/prefix/1", + }, + expectedService: &Service{ + Host: "example.com", + Port: 80, + Priority: 1, + Weight: 10, + Text: "hello", + Key: "/prefix/1", + }, + }, + { + name: "success with 'owned-by' (creation) without strictly owned", + ownerID: "owned-by", + exists: false, + service: &Service{ + Host: "example.com", + Port: 80, + Priority: 1, + Weight: 10, + Text: "hello", + Key: "/prefix/1", + }, + expectedService: &Service{ + Host: "example.com", + Port: 80, + Priority: 1, + Weight: 10, + Text: "hello", + Key: "/prefix/1", + }, + }, + { + name: "success with 'owned-by' (update) without strictly owned (owner not changed)", + ownerID: "owned-by", + exists: true, + service: &Service{ + Host: "example.com", + Port: 80, + Priority: 1, + Weight: 10, + Text: "hello", + Key: "/prefix/1", + OwnedBy: "owned-by", + }, + expectedService: &Service{ + Host: "example.com", + Port: 80, + Priority: 1, + Weight: 10, + Text: "hello", + Key: "/prefix/1", + OwnedBy: "owned-by", + }, + }, + { + name: "success with different 'owned-by' without strictly owned", + ownerID: "owned-by", + exists: true, + service: &Service{ + Host: "example.com", + Port: 80, + Priority: 1, + Weight: 10, + Text: "hello", + Key: "/prefix/1", + OwnedBy: "other-owned-by", + }, + expectedService: &Service{ + Host: "example.com", + Port: 80, + Priority: 1, + Weight: 10, + Text: "hello", + Key: "/prefix/1", + OwnedBy: "other-owned-by", + }, + }, + { + name: "failed with 'owned-by' is empty with strictly owned", + ownerID: "owned-by", + strictlyOwned: true, + exists: true, + service: &Service{ + Host: "example.com", + Port: 80, + Priority: 1, + Weight: 10, + Text: "hello", + Key: "/prefix/1", + }, + wantErr: true, + }, + { + name: "success with 'owned-by' (creation) with strictly owned", + ownerID: "owned-by", + strictlyOwned: true, + exists: false, + service: &Service{ + Host: "example.com", + Port: 80, + Priority: 1, + Weight: 10, + Text: "hello", + Key: "/prefix/1", + }, + expectedService: &Service{ + Host: "example.com", + Port: 80, + Priority: 1, + Weight: 10, + Text: "hello", + Key: "/prefix/1", + OwnedBy: "owned-by", + }, + }, + { + name: "success with 'owned-by' (update) with strictly owned (owner not changed)", + ownerID: "owned-by", + strictlyOwned: true, + exists: true, + ignoreGetCall: true, + service: &Service{ + Host: "example.com", + Port: 80, + Priority: 1, + Weight: 10, + Text: "hello", + Key: "/prefix/1", + OwnedBy: "owned-by", + }, + expectedService: &Service{ + Host: "example.com", + Port: 80, + Priority: 1, + Weight: 10, + Text: "hello", + Key: "/prefix/1", + OwnedBy: "owned-by", + }, + }, + { + name: "failed with different 'owned-by' with strictly owned", + ownerID: "owned-by", + strictlyOwned: true, + exists: true, + service: &Service{ + Host: "example.com", + Port: 80, + Priority: 1, + Weight: 10, + Text: "hello", + Key: "/prefix/1", + OwnedBy: "other-owned-by", + }, + wantErr: true, }, { name: "etcd put error", @@ -689,6 +1105,10 @@ func TestSaveService(t *testing.T) { Host: "example.com", Key: "/prefix/2", }, + expectedService: &Service{ + Host: "example.com", + Key: "/prefix/2", + }, mockPutErr: errors.New("etcd failure"), wantErr: true, }, @@ -697,15 +1117,37 @@ func TestSaveService(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { mockKV := new(MockEtcdKV) - value, err := json.Marshal(&tt.service) + value, err := json.Marshal(&tt.expectedService) require.NoError(t, err) - mockKV.On("Put", mock.Anything, tt.service.Key, string(value)). - Return(&etcdcv3.PutResponse{}, tt.mockPutErr) + if tt.expectedService != nil { + mockKV.On("Put", mock.Anything, tt.service.Key, string(value)). + Return(&etcdcv3.PutResponse{}, tt.mockPutErr) + } + actualValue, err := json.Marshal(&tt.service) + require.NoError(t, err) + if tt.strictlyOwned && !tt.ignoreGetCall { + if tt.exists { + mockKV.On("Get", mock.Anything, tt.service.Key).Return(&etcdcv3.GetResponse{ + Kvs: []*mvccpb.KeyValue{ + { + Key: []byte(tt.service.Key), + Value: actualValue, + }, + }, + }, nil) + } else { + mockKV.On("Get", mock.Anything, tt.service.Key).Return(&etcdcv3.GetResponse{ + Kvs: []*mvccpb.KeyValue{}, + }, nil) + } + } c := etcdClient{ client: &etcdcv3.Client{ KV: mockKV, }, + ownerID: tt.ownerID, + strictlyOwned: tt.strictlyOwned, } err = c.SaveService(context.Background(), tt.service) @@ -746,7 +1188,7 @@ func TestNewCoreDNSProvider(t *testing.T) { t.Run(tt.name, func(t *testing.T) { testutils.TestHelperEnvSetter(t, tt.envs) - provider, err := NewCoreDNSProvider(&endpoint.DomainFilter{}, "/prefix/", false) + provider, err := NewCoreDNSProvider(&endpoint.DomainFilter{}, "/prefix/", "", false, false) if tt.wantErr { require.Error(t, err) assert.EqualError(t, err, tt.errMsg)