From 1a48cb8e165232ee16a465afb5bc51d26d64b549 Mon Sep 17 00:00:00 2001 From: Romain Beuque <556072+rbeuque74@users.noreply.github.com> Date: Wed, 14 Sep 2022 10:06:54 +0000 Subject: [PATCH] feat(ovh): add cache based on DNS zone SOA value OVHcloud customers using 'ovh' provider for large DNS zone will spend a lot of time waiting for all records to be retrieved from the API. As records can't change without a modification of the SOA Serial of the DNS zone, this commit adds a client-side cache that stores the value of the DNS records for each zone, based of the current SOA Serial value of the zone. In case the SOA Serial of the zone is identical to the one in cache, we can serve all the records directly from cache. Otherwise, fallback on refreshing the cache from the API. Signed-off-by: Romain Beuque <556072+rbeuque74@users.noreply.github.com> --- docs/tutorials/ovh.md | 7 +- go.mod | 2 +- provider/ovh/ovh.go | 65 +++++++++++++++++ provider/ovh/ovh_test.go | 147 +++++++++++++++++++++++++++++++++++++-- 4 files changed, 214 insertions(+), 7 deletions(-) diff --git a/docs/tutorials/ovh.md b/docs/tutorials/ovh.md index 5bd5078e55..fbf7ee503a 100644 --- a/docs/tutorials/ovh.md +++ b/docs/tutorials/ovh.md @@ -18,7 +18,7 @@ instructions for creating a zone. You first need to create an OVH application. -Using the [OVH documentation](https://docs.ovh.com/gb/en/customer/first-steps-with-ovh-api/#creation-of-your-application-keys) you will have your `Application key` and `Application secret` +Using the [OVH documentation](https://docs.ovh.com/gb/en/api/first-steps-with-ovh-api/#advanced-usage-pair-ovhcloud-apis-with-an-application_2) you will have your `Application key` and `Application secret` And you will need to generate your consumer key, here the permissions needed : - GET on `/domain/zone` @@ -26,6 +26,7 @@ And you will need to generate your consumer key, here the permissions needed : - GET on `/domain/zone/*/record/*` - POST on `/domain/zone/*/record` - DELETE on `/domain/zone/*/record/*` +- GET on `/domain/zone/*/soa` - POST on `/domain/zone/*/refresh` You can use the following `curl` request to generate & validated your `Consumer key` @@ -37,6 +38,10 @@ curl -XPOST -H "X-Ovh-Application: " -H "Content-type: applicati "method": "GET", "path": "/domain/zone" }, + { + "method": "GET", + "path": "/domain/zone/*/soa" + }, { "method": "GET", "path": "/domain/zone/*/record" diff --git a/go.mod b/go.mod index 93125188a2..1445ce21f1 100644 --- a/go.mod +++ b/go.mod @@ -44,6 +44,7 @@ require ( github.com/openshift/client-go v0.0.0-20230607134213-3cd0021bbee3 github.com/oracle/oci-go-sdk/v65 v65.41.0 github.com/ovh/go-ovh v1.4.1 + github.com/patrickmn/go-cache v2.1.0+incompatible github.com/pkg/errors v0.9.1 github.com/pluralsh/gqlclient v1.5.1 github.com/projectcontour/contour v1.25.0 @@ -157,7 +158,6 @@ require ( github.com/oklog/ulid v1.3.1 // indirect github.com/openshift/gssapi v0.0.0-20161010215902-5fb4217df13b // indirect github.com/opentracing/opentracing-go v1.2.1-0.20220228012449-10b1cf09e00b // indirect - github.com/patrickmn/go-cache v2.1.0+incompatible // indirect github.com/pelletier/go-toml/v2 v2.0.6 // indirect github.com/peterhellberg/link v1.1.0 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect diff --git a/provider/ovh/ovh.go b/provider/ovh/ovh.go index 8cdb424ae3..d75d343bc5 100644 --- a/provider/ovh/ovh.go +++ b/provider/ovh/ovh.go @@ -21,12 +21,16 @@ import ( "errors" "fmt" "strings" + "time" + "github.com/miekg/dns" "github.com/ovh/go-ovh/ovh" + "github.com/patrickmn/go-cache" log "github.com/sirupsen/logrus" "golang.org/x/sync/errgroup" "sigs.k8s.io/external-dns/endpoint" + "sigs.k8s.io/external-dns/pkg/apis/externaldns" "sigs.k8s.io/external-dns/plan" "sigs.k8s.io/external-dns/provider" @@ -56,6 +60,17 @@ type OVHProvider struct { domainFilter endpoint.DomainFilter DryRun bool + + // UseCache controls if the OVHProvider will cache records in memory, and serve them + // without recontacting the OVHcloud API if the SOA of the domain zone hasn't changed. + // Note that, when disabling cache, OVHcloud API has rate-limiting that will hit if + // your refresh rate/number of records is too big, which might cause issue with the + // provider. + // Default value: true + UseCache bool + + cacheInstance *cache.Cache + dnsClient dnsClient } type ovhClient interface { @@ -64,6 +79,10 @@ type ovhClient interface { Delete(string, interface{}) error } +type dnsClient interface { + ExchangeContext(ctx context.Context, m *dns.Msg, a string) (*dns.Msg, time.Duration, error) +} + type ovhRecordFields struct { FieldType string `json:"fieldType"` SubDomain string `json:"subDomain"` @@ -88,6 +107,9 @@ func NewOVHProvider(ctx context.Context, domainFilter endpoint.DomainFilter, end if err != nil { return nil, err } + + client.UserAgent = externaldns.Version + // TODO: Add Dry Run support if dryRun { return nil, ErrNoDryRun @@ -97,6 +119,9 @@ func NewOVHProvider(ctx context.Context, domainFilter endpoint.DomainFilter, end domainFilter: domainFilter, apiRateLimiter: ratelimit.New(apiRateLimit), DryRun: dryRun, + cacheInstance: cache.New(cache.NoExpiration, cache.NoExpiration), + dnsClient: new(dns.Client), + UseCache: true, }, nil } @@ -217,14 +242,48 @@ func (p *OVHProvider) zones() ([]string, error) { return filteredZones, nil } +type ovhSoa struct { + Server string `json:"server"` + Serial uint32 `json:"serial"` + records []ovhRecord +} + func (p *OVHProvider) records(ctx *context.Context, zone *string, records chan<- []ovhRecord) error { var recordsIds []uint64 ovhRecords := make([]ovhRecord, len(recordsIds)) eg, _ := errgroup.WithContext(*ctx) + if p.UseCache { + if cachedSoaItf, ok := p.cacheInstance.Get(*zone + "#soa"); ok { + cachedSoa := cachedSoaItf.(ovhSoa) + + m := new(dns.Msg) + m.SetQuestion(dns.Fqdn(*zone), dns.TypeSOA) + in, _, err := p.dnsClient.ExchangeContext(*ctx, m, strings.TrimSuffix(cachedSoa.Server, ".")+":53") + if err == nil { + if s, ok := in.Answer[0].(*dns.SOA); ok { + // do something with t.Txt + if s.Serial == cachedSoa.Serial { + records <- cachedSoa.records + return nil + } + } + } + + p.cacheInstance.Delete(*zone + "#soa") + } + } + log.Debugf("OVH: Getting records for %s", *zone) p.apiRateLimiter.Take() + var soa ovhSoa + if p.UseCache { + if err := p.client.Get("/domain/zone/"+*zone+"/soa", &soa); err != nil { + return err + } + } + if err := p.client.Get(fmt.Sprintf("/domain/zone/%s/record", *zone), &recordsIds); err != nil { return err } @@ -240,6 +299,12 @@ func (p *OVHProvider) records(ctx *context.Context, zone *string, records chan<- for record := range chRecords { ovhRecords = append(ovhRecords, record) } + + if p.UseCache { + soa.records = ovhRecords + _ = p.cacheInstance.Add(*zone+"#soa", soa, time.Hour) + } + records <- ovhRecords return nil } diff --git a/provider/ovh/ovh_test.go b/provider/ovh/ovh_test.go index 6e925fcd6f..9aea3ad85f 100644 --- a/provider/ovh/ovh_test.go +++ b/provider/ovh/ovh_test.go @@ -19,10 +19,14 @@ package ovh import ( "context" "encoding/json" + "errors" "sort" "testing" + "time" + "github.com/miekg/dns" "github.com/ovh/go-ovh/ovh" + "github.com/patrickmn/go-cache" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "go.uber.org/ratelimit" @@ -55,6 +59,19 @@ func (c *mockOvhClient) Delete(endpoint string, output interface{}) error { return stub.Error(1) } +type mockDnsClient struct { + mock.Mock +} + +func (c *mockDnsClient) ExchangeContext(ctx context.Context, m *dns.Msg, addr string) (*dns.Msg, time.Duration, error) { + args := c.Called(ctx, m, addr) + + msg := args.Get(0).(*dns.Msg) + err := args.Error(1) + + return msg, time.Duration(0), err +} + func TestOvhZones(t *testing.T) { assert := assert.New(t) client := new(mockOvhClient) @@ -62,6 +79,8 @@ func TestOvhZones(t *testing.T) { client: client, apiRateLimiter: ratelimit.New(10), domainFilter: endpoint.NewDomainFilter([]string{"com"}), + cacheInstance: cache.New(cache.NoExpiration, cache.NoExpiration), + dnsClient: new(mockDnsClient), } // Basic zones @@ -83,10 +102,12 @@ func TestOvhZones(t *testing.T) { func TestOvhZoneRecords(t *testing.T) { assert := assert.New(t) client := new(mockOvhClient) - provider := &OVHProvider{client: client, apiRateLimiter: ratelimit.New(10)} + provider := &OVHProvider{client: client, apiRateLimiter: ratelimit.New(10), cacheInstance: cache.New(cache.NoExpiration, cache.NoExpiration), dnsClient: nil, UseCache: true} // Basic zones records + t.Log("Basic zones records") client.On("Get", "/domain/zone").Return([]string{"example.org"}, nil).Once() + client.On("Get", "/domain/zone/example.org/soa").Return(ovhSoa{Server: "ns.example.org.", Serial: 2022090901}, nil).Once() client.On("Get", "/domain/zone/example.org/record").Return([]uint64{24, 42}, nil).Once() client.On("Get", "/domain/zone/example.org/record/24").Return(ovhRecord{ID: 24, Zone: "example.org", ovhRecordFields: ovhRecordFields{SubDomain: "ovh", FieldType: "NS", TTL: 10, Target: "203.0.113.42"}}, nil).Once() client.On("Get", "/domain/zone/example.org/record/42").Return(ovhRecord{ID: 42, Zone: "example.org", ovhRecordFields: ovhRecordFields{SubDomain: "ovh", FieldType: "A", TTL: 10, Target: "203.0.113.42"}}, nil).Once() @@ -97,6 +118,7 @@ func TestOvhZoneRecords(t *testing.T) { client.AssertExpectations(t) // Error on getting zones list + t.Log("Error on getting zones list") client.On("Get", "/domain/zone").Return(nil, ovh.ErrAPIDown).Once() zones, records, err = provider.zonesRecords(context.TODO()) assert.Error(err) @@ -104,8 +126,21 @@ func TestOvhZoneRecords(t *testing.T) { assert.Nil(records) client.AssertExpectations(t) + // Error on getting zone SOA + t.Log("Error on getting zone SOA") + provider.cacheInstance = cache.New(cache.NoExpiration, cache.NoExpiration) + client.On("Get", "/domain/zone").Return([]string{"example.org"}, nil).Once() + client.On("Get", "/domain/zone/example.org/soa").Return(nil, ovh.ErrAPIDown).Once() + zones, records, err = provider.zonesRecords(context.TODO()) + assert.Error(err) + assert.Nil(zones) + assert.Nil(records) + client.AssertExpectations(t) + // Error on getting zone records + t.Log("Error on getting zone records") client.On("Get", "/domain/zone").Return([]string{"example.org"}, nil).Once() + client.On("Get", "/domain/zone/example.org/soa").Return(ovhSoa{Server: "ns.example.org.", Serial: 2022090902}, nil).Once() client.On("Get", "/domain/zone/example.org/record").Return(nil, ovh.ErrAPIDown).Once() zones, records, err = provider.zonesRecords(context.TODO()) assert.Error(err) @@ -114,7 +149,9 @@ func TestOvhZoneRecords(t *testing.T) { client.AssertExpectations(t) // Error on getting zone record detail + t.Log("Error on getting zone record detail") client.On("Get", "/domain/zone").Return([]string{"example.org"}, nil).Once() + client.On("Get", "/domain/zone/example.org/soa").Return(ovhSoa{Server: "ns.example.org.", Serial: 2022090902}, nil).Once() client.On("Get", "/domain/zone/example.org/record").Return([]uint64{42}, nil).Once() client.On("Get", "/domain/zone/example.org/record/42").Return(nil, ovh.ErrAPIDown).Once() zones, records, err = provider.zonesRecords(context.TODO()) @@ -124,10 +161,110 @@ func TestOvhZoneRecords(t *testing.T) { client.AssertExpectations(t) } +func TestOvhZoneRecordsCache(t *testing.T) { + assert := assert.New(t) + client := new(mockOvhClient) + dnsClient := new(mockDnsClient) + provider := &OVHProvider{client: client, apiRateLimiter: ratelimit.New(10), cacheInstance: cache.New(cache.NoExpiration, cache.NoExpiration), dnsClient: dnsClient, UseCache: true} + + // First call, cache miss + t.Log("First call, cache miss") + client.On("Get", "/domain/zone").Return([]string{"example.org"}, nil).Once() + client.On("Get", "/domain/zone/example.org/soa").Return(ovhSoa{Server: "ns.example.org.", Serial: 2022090901}, nil).Once() + client.On("Get", "/domain/zone/example.org/record").Return([]uint64{24, 42}, nil).Once() + client.On("Get", "/domain/zone/example.org/record/24").Return(ovhRecord{ID: 24, Zone: "example.org", ovhRecordFields: ovhRecordFields{SubDomain: "ovh", FieldType: "NS", TTL: 10, Target: "203.0.113.42"}}, nil).Once() + client.On("Get", "/domain/zone/example.org/record/42").Return(ovhRecord{ID: 42, Zone: "example.org", ovhRecordFields: ovhRecordFields{SubDomain: "ovh", FieldType: "A", TTL: 10, Target: "203.0.113.42"}}, nil).Once() + + zones, records, err := provider.zonesRecords(context.TODO()) + assert.NoError(err) + assert.ElementsMatch(zones, []string{"example.org"}) + assert.ElementsMatch(records, []ovhRecord{{ID: 42, Zone: "example.org", ovhRecordFields: ovhRecordFields{SubDomain: "ovh", FieldType: "A", TTL: 10, Target: "203.0.113.42"}}, {ID: 24, Zone: "example.org", ovhRecordFields: ovhRecordFields{SubDomain: "ovh", FieldType: "NS", TTL: 10, Target: "203.0.113.42"}}}) + client.AssertExpectations(t) + dnsClient.AssertExpectations(t) + + // reset mock + client = new(mockOvhClient) + dnsClient = new(mockDnsClient) + provider.client, provider.dnsClient = client, dnsClient + + // second call, cache hit + t.Log("second call, cache hit") + client.On("Get", "/domain/zone").Return([]string{"example.org"}, nil).Once() + dnsClient.On("ExchangeContext", mock.AnythingOfType("*context.cancelCtx"), mock.AnythingOfType("*dns.Msg"), "ns.example.org:53"). + Return(&dns.Msg{Answer: []dns.RR{&dns.SOA{Serial: 2022090901}}}, nil) + zones, records, err = provider.zonesRecords(context.TODO()) + assert.NoError(err) + assert.ElementsMatch(zones, []string{"example.org"}) + assert.ElementsMatch(records, []ovhRecord{{ID: 42, Zone: "example.org", ovhRecordFields: ovhRecordFields{SubDomain: "ovh", FieldType: "A", TTL: 10, Target: "203.0.113.42"}}, {ID: 24, Zone: "example.org", ovhRecordFields: ovhRecordFields{SubDomain: "ovh", FieldType: "NS", TTL: 10, Target: "203.0.113.42"}}}) + client.AssertExpectations(t) + dnsClient.AssertExpectations(t) + + // reset mock + client = new(mockOvhClient) + dnsClient = new(mockDnsClient) + provider.client, provider.dnsClient = client, dnsClient + + // third call, cache out of date + t.Log("third call, cache out of date") + client.On("Get", "/domain/zone").Return([]string{"example.org"}, nil).Once() + dnsClient.On("ExchangeContext", mock.AnythingOfType("*context.cancelCtx"), mock.AnythingOfType("*dns.Msg"), "ns.example.org:53"). + Return(&dns.Msg{Answer: []dns.RR{&dns.SOA{Serial: 2022090902}}}, nil) + client.On("Get", "/domain/zone/example.org/soa").Return(ovhSoa{Server: "ns.example.org.", Serial: 2022090902}, nil).Once() + client.On("Get", "/domain/zone/example.org/record").Return([]uint64{24}, nil).Once() + client.On("Get", "/domain/zone/example.org/record/24").Return(ovhRecord{ID: 24, Zone: "example.org", ovhRecordFields: ovhRecordFields{SubDomain: "ovh", FieldType: "NS", TTL: 10, Target: "203.0.113.42"}}, nil).Once() + + zones, records, err = provider.zonesRecords(context.TODO()) + assert.NoError(err) + assert.ElementsMatch(zones, []string{"example.org"}) + assert.ElementsMatch(records, []ovhRecord{{ID: 24, Zone: "example.org", ovhRecordFields: ovhRecordFields{SubDomain: "ovh", FieldType: "NS", TTL: 10, Target: "203.0.113.42"}}}) + client.AssertExpectations(t) + dnsClient.AssertExpectations(t) + + // reset mock + client = new(mockOvhClient) + dnsClient = new(mockDnsClient) + provider.client, provider.dnsClient = client, dnsClient + + // fourth call, cache hit + t.Log("fourth call, cache hit") + client.On("Get", "/domain/zone").Return([]string{"example.org"}, nil).Once() + dnsClient.On("ExchangeContext", mock.AnythingOfType("*context.cancelCtx"), mock.AnythingOfType("*dns.Msg"), "ns.example.org:53"). + Return(&dns.Msg{Answer: []dns.RR{&dns.SOA{Serial: 2022090902}}}, nil) + + zones, records, err = provider.zonesRecords(context.TODO()) + assert.NoError(err) + assert.ElementsMatch(zones, []string{"example.org"}) + assert.ElementsMatch(records, []ovhRecord{{ID: 24, Zone: "example.org", ovhRecordFields: ovhRecordFields{SubDomain: "ovh", FieldType: "NS", TTL: 10, Target: "203.0.113.42"}}}) + client.AssertExpectations(t) + dnsClient.AssertExpectations(t) + + // reset mock + client = new(mockOvhClient) + dnsClient = new(mockDnsClient) + provider.client, provider.dnsClient = client, dnsClient + + // fifth call, dns issue + t.Log("fourth call, cache hit") + client.On("Get", "/domain/zone").Return([]string{"example.org"}, nil).Once() + dnsClient.On("ExchangeContext", mock.AnythingOfType("*context.cancelCtx"), mock.AnythingOfType("*dns.Msg"), "ns.example.org:53"). + Return(&dns.Msg{Answer: []dns.RR{}}, errors.New("dns issue")) + client.On("Get", "/domain/zone/example.org/soa").Return(ovhSoa{Server: "ns.example.org.", Serial: 2022090903}, nil).Once() + client.On("Get", "/domain/zone/example.org/record").Return([]uint64{24, 42}, nil).Once() + client.On("Get", "/domain/zone/example.org/record/24").Return(ovhRecord{ID: 24, Zone: "example.org", ovhRecordFields: ovhRecordFields{SubDomain: "ovh", FieldType: "NS", TTL: 10, Target: "203.0.113.42"}}, nil).Once() + client.On("Get", "/domain/zone/example.org/record/42").Return(ovhRecord{ID: 42, Zone: "example.org", ovhRecordFields: ovhRecordFields{SubDomain: "ovh", FieldType: "A", TTL: 10, Target: "203.0.113.42"}}, nil).Once() + + zones, records, err = provider.zonesRecords(context.TODO()) + assert.NoError(err) + assert.ElementsMatch(zones, []string{"example.org"}) + assert.ElementsMatch(records, []ovhRecord{{ID: 42, Zone: "example.org", ovhRecordFields: ovhRecordFields{SubDomain: "ovh", FieldType: "A", TTL: 10, Target: "203.0.113.42"}}, {ID: 24, Zone: "example.org", ovhRecordFields: ovhRecordFields{SubDomain: "ovh", FieldType: "NS", TTL: 10, Target: "203.0.113.42"}}}) + client.AssertExpectations(t) + dnsClient.AssertExpectations(t) +} + func TestOvhRecords(t *testing.T) { assert := assert.New(t) client := new(mockOvhClient) - provider := &OVHProvider{client: client, apiRateLimiter: ratelimit.New(10)} + provider := &OVHProvider{client: client, apiRateLimiter: ratelimit.New(10), cacheInstance: cache.New(cache.NoExpiration, cache.NoExpiration)} // Basic zones records client.On("Get", "/domain/zone").Return([]string{"example.org", "example.net"}, nil).Once() @@ -160,7 +297,7 @@ func TestOvhRecords(t *testing.T) { func TestOvhRefresh(t *testing.T) { client := new(mockOvhClient) - provider := &OVHProvider{client: client, apiRateLimiter: ratelimit.New(10)} + provider := &OVHProvider{client: client, apiRateLimiter: ratelimit.New(10), cacheInstance: cache.New(cache.NoExpiration, cache.NoExpiration)} // Basic zone refresh client.On("Post", "/domain/zone/example.net/refresh", nil).Return(nil, nil).Once() @@ -201,7 +338,7 @@ func TestOvhNewChange(t *testing.T) { func TestOvhApplyChanges(t *testing.T) { assert := assert.New(t) client := new(mockOvhClient) - provider := &OVHProvider{client: client, apiRateLimiter: ratelimit.New(10)} + provider := &OVHProvider{client: client, apiRateLimiter: ratelimit.New(10), cacheInstance: cache.New(cache.NoExpiration, cache.NoExpiration)} changes := plan.Changes{ Create: []*endpoint.Endpoint{ {DNSName: ".example.net", RecordType: "A", RecordTTL: 10, Targets: []string{"203.0.113.42"}}, @@ -254,7 +391,7 @@ func TestOvhApplyChanges(t *testing.T) { func TestOvhChange(t *testing.T) { assert := assert.New(t) client := new(mockOvhClient) - provider := &OVHProvider{client: client, apiRateLimiter: ratelimit.New(10)} + provider := &OVHProvider{client: client, apiRateLimiter: ratelimit.New(10), cacheInstance: cache.New(cache.NoExpiration, cache.NoExpiration)} // Record creation client.On("Post", "/domain/zone/example.net/record", ovhRecordFields{SubDomain: "ovh"}).Return(nil, nil).Once()