From 584d8fde704fda9b92ad8b91479833e3b47a5f30 Mon Sep 17 00:00:00 2001 From: Miciah Masters Date: Fri, 5 May 2023 21:00:40 -0400 Subject: [PATCH] Add support for AWS shared VPC in another account Add support for configuring DNS records in AWS Route 53 using a separate account for the private hosted zone. This commit resolves NE-1294. https://issues.redhat.com/browse/NE-1294 * pkg/dns/aws/dns.go (Config): Add a RoleARN field. (NewProvider): If config.RoleARN is set, use it to configure the AWS client using the specified role. * pkg/dns/split/dns.go: New file. Define a DNS provider implementation that wraps two other DNS providers, using one of them to publish records to the public zone and the other to publish records to the private zone. (Provider): New type. Store the private and public DNS providers, as well as the private zone so that the Ensure, Delete, and Replace methods can use it to determine whether they are publishing to the public zone or to the private zone. (NewProvider): New function. Return a split DNS provider. (Ensure, Delete, Replace): New methods. Implement the dns.Provider interface by calling the respective methods on the wrapped private and public DNS providers. * pkg/dns/split/dns_test.go (TestSplitDNSProvider): Verify that the split DNS provider correctly dispatches to the private or public DNS provider as appropriate, using fakeProvider. (fakeProvider): New type. Define a fake named DNS provider that records its name when invoked. (Ensure, Delete, Replace): New methods for fakeProvider to record invocations and implement the dns.Provider interface. (newFakeProvider): New function. Return a fake provider. * pkg/operator/controller/dns/controller.go (createDNSProvider): Use the new split DNS provider and the AWS DNS provider's new RoleARN configuration option to configure separate DNS providers for public and private zones when a role ARN for the private zone is specified in the cluster infrastructure config. --- pkg/dns/aws/dns.go | 7 ++ pkg/dns/split/dns.go | 58 ++++++++++ pkg/dns/split/dns_test.go | 126 ++++++++++++++++++++++ pkg/operator/controller/dns/controller.go | 17 ++- 4 files changed, 207 insertions(+), 1 deletion(-) create mode 100644 pkg/dns/split/dns.go create mode 100644 pkg/dns/split/dns_test.go diff --git a/pkg/dns/aws/dns.go b/pkg/dns/aws/dns.go index 5d321e007..2b6864496 100644 --- a/pkg/dns/aws/dns.go +++ b/pkg/dns/aws/dns.go @@ -16,6 +16,7 @@ import ( "github.com/aws/aws-sdk-go/aws/arn" "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/aws/client/metadata" + "github.com/aws/aws-sdk-go/aws/credentials/stscreds" "github.com/aws/aws-sdk-go/aws/endpoints" "github.com/aws/aws-sdk-go/aws/request" "github.com/aws/aws-sdk-go/aws/session" @@ -93,6 +94,9 @@ type Config struct { // that is used by SDK to configure the credentials. SharedCredentialFile string + // RoleARN is an optional ARN to use for the AWS client session. + RoleARN string + // Region is the AWS region ELBs are created in. Region string // ServiceEndpoints is the list of AWS API endpoints to use for @@ -140,6 +144,9 @@ func NewProvider(config Config, operatorReleaseVersion string) (*Provider, error Name: "openshift.io/ingress-operator", Fn: request.MakeAddToUserAgentHandler("openshift.io ingress-operator", operatorReleaseVersion), }) + if config.RoleARN != "" { + sess.Config.WithCredentials(stscreds.NewCredentials(sess, config.RoleARN)) + } if len(region) == 0 { if sess.Config.Region != nil { diff --git a/pkg/dns/split/dns.go b/pkg/dns/split/dns.go new file mode 100644 index 000000000..c055c43f1 --- /dev/null +++ b/pkg/dns/split/dns.go @@ -0,0 +1,58 @@ +package split + +import ( + "reflect" + + iov1 "github.com/openshift/api/operatoringress/v1" + "github.com/openshift/cluster-ingress-operator/pkg/dns" + logf "github.com/openshift/cluster-ingress-operator/pkg/log" + + configv1 "github.com/openshift/api/config/v1" +) + +var ( + _ dns.Provider = &Provider{} + log = logf.Logger.WithName("dns") +) + +// Provider is a dns.Provider that wraps two other providers. The first +// provider is used for public hosted zones, and the second provider is used for +// private hosted zones. +type Provider struct { + private, public dns.Provider + privateZone *configv1.DNSZone +} + +// NewProvider returns a new Provider that wraps the provided wrappers, using +// the first for the public zone and the second for the private zone. +func NewProvider(public, private dns.Provider, privateZone *configv1.DNSZone) *Provider { + return &Provider{ + public: public, + private: private, + privateZone: privateZone, + } +} + +// Ensure calls the Ensure method of one of the wrapped DNS providers. +func (p *Provider) Ensure(record *iov1.DNSRecord, zone configv1.DNSZone) error { + if reflect.DeepEqual(zone, *p.privateZone) { + return p.private.Ensure(record, zone) + } + return p.public.Ensure(record, zone) +} + +// Delete calls the Delete method of one of the wrapped DNS providers. +func (p *Provider) Delete(record *iov1.DNSRecord, zone configv1.DNSZone) error { + if reflect.DeepEqual(zone, *p.privateZone) { + return p.private.Delete(record, zone) + } + return p.public.Delete(record, zone) +} + +// Replace calls the Replace method of one of the wrapped DNS providers. +func (p *Provider) Replace(record *iov1.DNSRecord, zone configv1.DNSZone) error { + if reflect.DeepEqual(zone, *p.privateZone) { + return p.private.Replace(record, zone) + } + return p.public.Replace(record, zone) +} diff --git a/pkg/dns/split/dns_test.go b/pkg/dns/split/dns_test.go new file mode 100644 index 000000000..f2a669f83 --- /dev/null +++ b/pkg/dns/split/dns_test.go @@ -0,0 +1,126 @@ +package split_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + configv1 "github.com/openshift/api/config/v1" + iov1 "github.com/openshift/api/operatoringress/v1" + + "github.com/openshift/cluster-ingress-operator/pkg/dns" + splitdns "github.com/openshift/cluster-ingress-operator/pkg/dns/split" +) + +// TestSplitDNSProvider verifies that the split DNS provider dispatches to the +// public or private provider as appropriate for the DNS zone. +func TestSplitDNSProvider(t *testing.T) { + var ( + // ch is a channel that is used in the fake public and private + // providers to record which one is called. + ch = make(chan string, 6) + // getResult reads and returns one item from ch, or returns the + // empty string if ch is empty. + getResult = func() string { + var result string + select { + case result = <-ch: + default: + } + return result + } + // publicProvider is a fake dns.Provider for the public zone. + publicProvider = newFakeProvider("public", ch) + // privateProvider is a fake dns.Provider for the private zone. + privateProvider = newFakeProvider("private", ch) + // publicZoneWithID is a public zone that is defined by ID. + publicZoneWithID = configv1.DNSZone{ID: "public_zone"} + // privateZoneWithID is a private zone that is defined by ID. + privateZoneWithID = configv1.DNSZone{ID: "private_zone"} + // publicZoneWithTags is a public zone that is defined by tags. + publicZoneWithTags = configv1.DNSZone{Tags: map[string]string{"zone": "public"}} + // privateZoneWithID is a private zone that is defined by tags. + privateZoneWithTags = configv1.DNSZone{Tags: map[string]string{"zone": "private"}} + ) + testCases := []struct { + name string + publicZone configv1.DNSZone + privateZone configv1.DNSZone + publishToZone configv1.DNSZone + expect string + }{ + { + name: "publish to public zone specified by id", + publicZone: publicZoneWithID, + privateZone: privateZoneWithID, + publishToZone: publicZoneWithID, + expect: "public", + }, + { + name: "publish to private zone specified by id", + publicZone: publicZoneWithID, + privateZone: privateZoneWithID, + publishToZone: privateZoneWithID, + expect: "private", + }, + { + name: "publish to public zone specified by tags", + publicZone: publicZoneWithTags, + privateZone: privateZoneWithID, + publishToZone: publicZoneWithTags, + expect: "public", + }, + { + name: "publish to private zone specified by tags", + publicZone: publicZoneWithTags, + privateZone: privateZoneWithTags, + publishToZone: privateZoneWithTags, + expect: "private", + }, + { + name: "publish to other zone should fall back to the public zone", + publicZone: publicZoneWithID, + privateZone: privateZoneWithID, + publishToZone: configv1.DNSZone{ID: "other_zone"}, + expect: "public", + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + provider := splitdns.NewProvider(publicProvider, privateProvider, &tc.privateZone) + assert.NoError(t, provider.Ensure(&iov1.DNSRecord{}, tc.publishToZone)) + assert.Equal(t, tc.expect, getResult()) + assert.NoError(t, provider.Replace(&iov1.DNSRecord{}, tc.publishToZone)) + assert.Equal(t, tc.expect, getResult()) + assert.NoError(t, provider.Delete(&iov1.DNSRecord{}, tc.publishToZone)) + assert.Equal(t, tc.expect, getResult()) + assert.Empty(t, ch) + }) + } + +} + +var _ dns.Provider = &fakeProvider{} + +type fakeProvider struct { + name string + recorder chan string +} + +func (p *fakeProvider) Ensure(record *iov1.DNSRecord, zone configv1.DNSZone) error { + p.recorder <- p.name + return nil +} +func (p *fakeProvider) Delete(record *iov1.DNSRecord, zone configv1.DNSZone) error { + p.recorder <- p.name + return nil +} +func (p *fakeProvider) Replace(record *iov1.DNSRecord, zone configv1.DNSZone) error { + p.recorder <- p.name + return nil +} + +// newFakeProvider returns a new dns.Provider that records invocations. +func newFakeProvider(name string, ch chan string) dns.Provider { + return &fakeProvider{name, ch} +} diff --git a/pkg/operator/controller/dns/controller.go b/pkg/operator/controller/dns/controller.go index 6bd5494e9..06a2f4c54 100644 --- a/pkg/operator/controller/dns/controller.go +++ b/pkg/operator/controller/dns/controller.go @@ -22,6 +22,7 @@ import ( ibm "github.com/openshift/cluster-ingress-operator/pkg/dns/ibm" ibmprivatedns "github.com/openshift/cluster-ingress-operator/pkg/dns/ibm/private" ibmpublicdns "github.com/openshift/cluster-ingress-operator/pkg/dns/ibm/public" + splitdns "github.com/openshift/cluster-ingress-operator/pkg/dns/split" logf "github.com/openshift/cluster-ingress-operator/pkg/log" "github.com/openshift/cluster-ingress-operator/pkg/manifests" "github.com/openshift/cluster-ingress-operator/pkg/operator/controller" @@ -662,7 +663,21 @@ func (r *reconciler) createDNSProvider(dnsConfig *configv1.DNS, platformStatus * if err != nil { return nil, fmt.Errorf("failed to create AWS DNS manager: %v", err) } - dnsProvider = provider + var roleARN string + if dnsConfig.Spec.Platform.AWS != nil { + roleARN = dnsConfig.Spec.Platform.AWS.PrivateZoneIAMRole + } + if roleARN != "" { + cfg := cfg + cfg.RoleARN = roleARN + privateProvider, err := awsdns.NewProvider(cfg, r.config.OperatorReleaseVersion) + if err != nil { + return nil, fmt.Errorf("failed to create AWS DNS manager for shared VPC: %v", err) + } + dnsProvider = splitdns.NewProvider(provider, privateProvider, dnsConfig.Spec.PrivateZone) + } else { + dnsProvider = provider + } case configv1.AzurePlatformType: environment := platformStatus.Azure.CloudName if environment == "" {