From cedd117f62f59f109350f7a4337ef1a89f2dd4f4 Mon Sep 17 00:00:00 2001 From: Abhinav Dahiya Date: Wed, 13 May 2020 21:18:20 -0700 Subject: [PATCH 1/4] pkg/asset/installconfig/gcp: update go generate for moc generation update the destination of the generated mocks --- hack/go-genmock.sh | 2 +- .../gcp/.mock/gcpclient_generated.go | 111 ------------------ pkg/asset/installconfig/gcp/client.go | 2 +- 3 files changed, 2 insertions(+), 113 deletions(-) delete mode 100644 pkg/asset/installconfig/gcp/.mock/gcpclient_generated.go diff --git a/hack/go-genmock.sh b/hack/go-genmock.sh index 8214e8d68b5..37ca553b91c 100755 --- a/hack/go-genmock.sh +++ b/hack/go-genmock.sh @@ -2,7 +2,7 @@ # Example: ./hack/go-genmock.sh if [ "$IS_CONTAINER" != "" ]; then - go generate ./pkg/... "${@}" + go generate ./pkg/asset/installconfig/... "${@}" else podman build -t openshift-install-mock ./images/mock podman run --rm \ diff --git a/pkg/asset/installconfig/gcp/.mock/gcpclient_generated.go b/pkg/asset/installconfig/gcp/.mock/gcpclient_generated.go deleted file mode 100644 index 452e381bf57..00000000000 --- a/pkg/asset/installconfig/gcp/.mock/gcpclient_generated.go +++ /dev/null @@ -1,111 +0,0 @@ -// Code generated by MockGen. DO NOT EDIT. -// Source: ./client.go - -// Package mock is a generated GoMock package. -package mock - -import ( - context "context" - gomock "github.com/golang/mock/gomock" - compute "google.golang.org/api/compute/v1" - dns "google.golang.org/api/dns/v1" - reflect "reflect" -) - -// MockAPI is a mock of API interface. -type MockAPI struct { - ctrl *gomock.Controller - recorder *MockAPIMockRecorder -} - -// MockAPIMockRecorder is the mock recorder for MockAPI. -type MockAPIMockRecorder struct { - mock *MockAPI -} - -// NewMockAPI creates a new mock instance. -func NewMockAPI(ctrl *gomock.Controller) *MockAPI { - mock := &MockAPI{ctrl: ctrl} - mock.recorder = &MockAPIMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockAPI) EXPECT() *MockAPIMockRecorder { - return m.recorder -} - -// GetNetwork mocks base method. -func (m *MockAPI) GetNetwork(ctx context.Context, network, project string) (*compute.Network, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetNetwork", ctx, network, project) - ret0, _ := ret[0].(*compute.Network) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetNetwork indicates an expected call of GetNetwork. -func (mr *MockAPIMockRecorder) GetNetwork(ctx, network, project interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNetwork", reflect.TypeOf((*MockAPI)(nil).GetNetwork), ctx, network, project) -} - -// GetPublicDomains mocks base method. -func (m *MockAPI) GetPublicDomains(ctx context.Context, project string) ([]string, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetPublicDomains", ctx, project) - ret0, _ := ret[0].([]string) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetPublicDomains indicates an expected call of GetPublicDomains. -func (mr *MockAPIMockRecorder) GetPublicDomains(ctx, project interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPublicDomains", reflect.TypeOf((*MockAPI)(nil).GetPublicDomains), ctx, project) -} - -// GetPublicDNSZone mocks base method. -func (m *MockAPI) GetPublicDNSZone(ctx context.Context, baseDomain, project string) (*dns.ManagedZone, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetPublicDNSZone", ctx, baseDomain, project) - ret0, _ := ret[0].(*dns.ManagedZone) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetPublicDNSZone indicates an expected call of GetPublicDNSZone. -func (mr *MockAPIMockRecorder) GetPublicDNSZone(ctx, baseDomain, project interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPublicDNSZone", reflect.TypeOf((*MockAPI)(nil).GetPublicDNSZone), ctx, baseDomain, project) -} - -// GetSubnetworks mocks base method. -func (m *MockAPI) GetSubnetworks(ctx context.Context, network, project, region string) ([]*compute.Subnetwork, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetSubnetworks", ctx, network, project, region) - ret0, _ := ret[0].([]*compute.Subnetwork) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetSubnetworks indicates an expected call of GetSubnetworks. -func (mr *MockAPIMockRecorder) GetSubnetworks(ctx, network, project, region interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSubnetworks", reflect.TypeOf((*MockAPI)(nil).GetSubnetworks), ctx, network, project, region) -} - -// GetListOfProjects mocks base method. -func (m *MockAPI) GetListOfProjects(ctx context.Context) (map[string]string, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetListOfProjects", ctx) - ret0, _ := ret[0].(map[string]string) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetListOfProjects indicates an expected call of GetListOfProjects. -func (mr *MockAPIMockRecorder) GetListOfProjects(ctx interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetListOfProjects", reflect.TypeOf((*MockAPI)(nil).GetListOfProjects), ctx) -} diff --git a/pkg/asset/installconfig/gcp/client.go b/pkg/asset/installconfig/gcp/client.go index dd19d5587f5..29659080851 100644 --- a/pkg/asset/installconfig/gcp/client.go +++ b/pkg/asset/installconfig/gcp/client.go @@ -13,7 +13,7 @@ import ( "google.golang.org/api/option" ) -//go:generate mockgen -source=./client.go -destination=.mock/gcpclient_generated.go -package=mock +//go:generate mockgen -source=./client.go -destination=./mock/gcpclient_generated.go -package=mock // API represents the calls made to the API. type API interface { From 311a2e9c94948f7ab3774ee1b869e5a6f5050448 Mon Sep 17 00:00:00 2001 From: Abhinav Dahiya Date: Wed, 13 May 2020 21:20:41 -0700 Subject: [PATCH 2/4] pkg/asset/installconfig/gcp: add function to validate pre-existing api dns record For external clusters, we need to make sure there are no pre existing `api.` dns records. To achieve that, we find the public dns zone for the basedomain and list all the dns records. Checking for `api.` in the retrieved list shuold allow failing early. --- pkg/asset/installconfig/gcp/client.go | 22 +++++++ .../gcp/mock/gcpclient_generated.go | 61 ++++++++++++------- pkg/asset/installconfig/gcp/validation.go | 39 ++++++++++++ .../installconfig/gcp/validation_test.go | 46 ++++++++++++++ 4 files changed, 145 insertions(+), 23 deletions(-) diff --git a/pkg/asset/installconfig/gcp/client.go b/pkg/asset/installconfig/gcp/client.go index 29659080851..937c1632297 100644 --- a/pkg/asset/installconfig/gcp/client.go +++ b/pkg/asset/installconfig/gcp/client.go @@ -22,6 +22,7 @@ type API interface { GetPublicDNSZone(ctx context.Context, baseDomain, project string) (*dns.ManagedZone, error) GetSubnetworks(ctx context.Context, network, project, region string) ([]*compute.Subnetwork, error) GetProjects(ctx context.Context) (map[string]string, error) + GetRecordSets(ctx context.Context, project, zone string) ([]*dns.ResourceRecordSet, error) } // Client makes calls to the GCP API. @@ -114,6 +115,27 @@ func (c *Client) GetPublicDNSZone(ctx context.Context, project, baseDomain strin return res, nil } +// GetRecordSets returns all the records for a DNS zone. +func (c *Client) GetRecordSets(ctx context.Context, project, zone string) ([]*dns.ResourceRecordSet, error) { + ctx, cancel := context.WithTimeout(context.TODO(), 1*time.Minute) + defer cancel() + + svc, err := c.getDNSService(ctx) + if err != nil { + return nil, err + } + + req := svc.ResourceRecordSets.List(project, zone).Context(ctx) + var rrSets []*dns.ResourceRecordSet + if err := req.Pages(ctx, func(page *dns.ResourceRecordSetsListResponse) error { + rrSets = append(rrSets, page.Rrsets...) + return nil + }); err != nil { + return nil, err + } + return rrSets, nil +} + // GetSubnetworks uses the GCP Compute Service API to retrieve all subnetworks in a given network. func (c *Client) GetSubnetworks(ctx context.Context, network, project, region string) ([]*compute.Subnetwork, error) { ctx, cancel := context.WithTimeout(ctx, 1*time.Minute) diff --git a/pkg/asset/installconfig/gcp/mock/gcpclient_generated.go b/pkg/asset/installconfig/gcp/mock/gcpclient_generated.go index dce3baeb7e6..15a6649b0ab 100644 --- a/pkg/asset/installconfig/gcp/mock/gcpclient_generated.go +++ b/pkg/asset/installconfig/gcp/mock/gcpclient_generated.go @@ -1,5 +1,5 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: pkg/asset/installconfig/gcp/client.go +// Source: ./client.go // Package mock is a generated GoMock package. package mock @@ -7,50 +7,50 @@ package mock import ( context "context" gomock "github.com/golang/mock/gomock" - v1 "google.golang.org/api/compute/v1" - v10 "google.golang.org/api/dns/v1" + compute "google.golang.org/api/compute/v1" + dns "google.golang.org/api/dns/v1" reflect "reflect" ) -// MockAPI is a mock of API interface +// MockAPI is a mock of API interface. type MockAPI struct { ctrl *gomock.Controller recorder *MockAPIMockRecorder } -// MockAPIMockRecorder is the mock recorder for MockAPI +// MockAPIMockRecorder is the mock recorder for MockAPI. type MockAPIMockRecorder struct { mock *MockAPI } -// NewMockAPI creates a new mock instance +// NewMockAPI creates a new mock instance. func NewMockAPI(ctrl *gomock.Controller) *MockAPI { mock := &MockAPI{ctrl: ctrl} mock.recorder = &MockAPIMockRecorder{mock} return mock } -// EXPECT returns an object that allows the caller to indicate expected use +// EXPECT returns an object that allows the caller to indicate expected use. func (m *MockAPI) EXPECT() *MockAPIMockRecorder { return m.recorder } -// GetNetwork mocks base method -func (m *MockAPI) GetNetwork(ctx context.Context, network, project string) (*v1.Network, error) { +// GetNetwork mocks base method. +func (m *MockAPI) GetNetwork(ctx context.Context, network, project string) (*compute.Network, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetNetwork", ctx, network, project) - ret0, _ := ret[0].(*v1.Network) + ret0, _ := ret[0].(*compute.Network) ret1, _ := ret[1].(error) return ret0, ret1 } -// GetNetwork indicates an expected call of GetNetwork +// GetNetwork indicates an expected call of GetNetwork. func (mr *MockAPIMockRecorder) GetNetwork(ctx, network, project interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNetwork", reflect.TypeOf((*MockAPI)(nil).GetNetwork), ctx, network, project) } -// GetPublicDomains mocks base method +// GetPublicDomains mocks base method. func (m *MockAPI) GetPublicDomains(ctx context.Context, project string) ([]string, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetPublicDomains", ctx, project) @@ -59,43 +59,43 @@ func (m *MockAPI) GetPublicDomains(ctx context.Context, project string) ([]strin return ret0, ret1 } -// GetPublicDomains indicates an expected call of GetPublicDomains +// GetPublicDomains indicates an expected call of GetPublicDomains. func (mr *MockAPIMockRecorder) GetPublicDomains(ctx, project interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPublicDomains", reflect.TypeOf((*MockAPI)(nil).GetPublicDomains), ctx, project) } -// GetPublicDNSZone mocks base method -func (m *MockAPI) GetPublicDNSZone(ctx context.Context, baseDomain, project string) (*v10.ManagedZone, error) { +// GetPublicDNSZone mocks base method. +func (m *MockAPI) GetPublicDNSZone(ctx context.Context, baseDomain, project string) (*dns.ManagedZone, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetPublicDNSZone", ctx, baseDomain, project) - ret0, _ := ret[0].(*v10.ManagedZone) + ret0, _ := ret[0].(*dns.ManagedZone) ret1, _ := ret[1].(error) return ret0, ret1 } -// GetPublicDNSZone indicates an expected call of GetPublicDNSZone +// GetPublicDNSZone indicates an expected call of GetPublicDNSZone. func (mr *MockAPIMockRecorder) GetPublicDNSZone(ctx, baseDomain, project interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPublicDNSZone", reflect.TypeOf((*MockAPI)(nil).GetPublicDNSZone), ctx, baseDomain, project) } -// GetSubnetworks mocks base method -func (m *MockAPI) GetSubnetworks(ctx context.Context, network, project, region string) ([]*v1.Subnetwork, error) { +// GetSubnetworks mocks base method. +func (m *MockAPI) GetSubnetworks(ctx context.Context, network, project, region string) ([]*compute.Subnetwork, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetSubnetworks", ctx, network, project, region) - ret0, _ := ret[0].([]*v1.Subnetwork) + ret0, _ := ret[0].([]*compute.Subnetwork) ret1, _ := ret[1].(error) return ret0, ret1 } -// GetSubnetworks indicates an expected call of GetSubnetworks +// GetSubnetworks indicates an expected call of GetSubnetworks. func (mr *MockAPIMockRecorder) GetSubnetworks(ctx, network, project, region interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSubnetworks", reflect.TypeOf((*MockAPI)(nil).GetSubnetworks), ctx, network, project, region) } -// GetProjects mocks base method +// GetProjects mocks base method. func (m *MockAPI) GetProjects(ctx context.Context) (map[string]string, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetProjects", ctx) @@ -104,8 +104,23 @@ func (m *MockAPI) GetProjects(ctx context.Context) (map[string]string, error) { return ret0, ret1 } -// GetProjects indicates an expected call of GetProjects +// GetProjects indicates an expected call of GetProjects. func (mr *MockAPIMockRecorder) GetProjects(ctx interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetProjects", reflect.TypeOf((*MockAPI)(nil).GetProjects), ctx) } + +// GetRecordSets mocks base method. +func (m *MockAPI) GetRecordSets(ctx context.Context, project, zone string) ([]*dns.ResourceRecordSet, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetRecordSets", ctx, project, zone) + ret0, _ := ret[0].([]*dns.ResourceRecordSet) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetRecordSets indicates an expected call of GetRecordSets. +func (mr *MockAPIMockRecorder) GetRecordSets(ctx, project, zone interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRecordSets", reflect.TypeOf((*MockAPI)(nil).GetRecordSets), ctx, project, zone) +} diff --git a/pkg/asset/installconfig/gcp/validation.go b/pkg/asset/installconfig/gcp/validation.go index 25e3b8d22fe..163fb895ab9 100644 --- a/pkg/asset/installconfig/gcp/validation.go +++ b/pkg/asset/installconfig/gcp/validation.go @@ -2,10 +2,14 @@ package gcp import ( "context" + "errors" "fmt" "net" + "net/http" + "strings" compute "google.golang.org/api/compute/v1" + "google.golang.org/api/googleapi" "k8s.io/apimachinery/pkg/util/validation/field" "github.com/openshift/installer/pkg/types" @@ -21,6 +25,41 @@ func Validate(client API, ic *types.InstallConfig) error { return allErrs.ToAggregate() } +// ValidatePreExitingPublicDNS ensure no pre-existing DNS record exists in the public +// DNS zone for cluster's Kubernetes API. +func ValidatePreExitingPublicDNS(client API, ic *types.InstallConfig) error { + // If this is an internal cluster, this check is not necessary + if ic.Publish == types.InternalPublishingStrategy { + return nil + } + + record := fmt.Sprintf("api.%s.", strings.TrimSuffix(ic.ClusterDomain(), ".")) + + zone, err := client.GetPublicDNSZone(context.TODO(), ic.BaseDomain, ic.Platform.GCP.ProjectID) + if err != nil { + var gErr *googleapi.Error + if errors.As(err, &gErr) { + if gErr.Code == http.StatusNotFound { + return field.NotFound(field.NewPath("baseDomain"), fmt.Sprintf("DNS Zone (%s/%s)", ic.Platform.GCP.ProjectID, ic.BaseDomain)) + } + } + return field.InternalError(field.NewPath("baseDomain"), err) + } + + rrSets, err := client.GetRecordSets(context.TODO(), ic.Platform.GCP.ProjectID, zone.Name) + if err != nil { + return field.InternalError(field.NewPath("baseDomain"), err) + } + + for _, r := range rrSets { + if strings.EqualFold(r.Name, record) { + return field.Invalid(field.NewPath("metadata", "name"), ic.ObjectMeta.Name, fmt.Sprintf("record %s already exists in DNS Zone (%s/%s) and might be in use by another cluster, please remove it to continue", record, ic.Platform.GCP.ProjectID, zone.Name)) + } + } + + return nil +} + func validateProject(client API, ic *types.InstallConfig, fieldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} diff --git a/pkg/asset/installconfig/gcp/validation_test.go b/pkg/asset/installconfig/gcp/validation_test.go index 0f1922c7b2d..ecc81e07338 100644 --- a/pkg/asset/installconfig/gcp/validation_test.go +++ b/pkg/asset/installconfig/gcp/validation_test.go @@ -8,6 +8,8 @@ import ( "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" compute "google.golang.org/api/compute/v1" + dns "google.golang.org/api/dns/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/openshift/installer/pkg/asset/installconfig/gcp/mock" "github.com/openshift/installer/pkg/ipnet" @@ -183,3 +185,47 @@ func TestGCPInstallConfigValidation(t *testing.T) { }) } } + +func TestValidatePreExitingPublicDNS(t *testing.T) { + cases := []struct { + name string + records []*dns.ResourceRecordSet + err string + }{{ + name: "no pre-existing", + records: nil, + }, { + name: "no pre-existing", + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.base-domain."}}, + }, { + name: "pre-existing", + records: []*dns.ResourceRecordSet{{Name: "api.cluster-name.base-domain."}}, + err: `^metadata\.name: Invalid value: "cluster-name": record api\.cluster-name\.base-domain\. already exists in DNS Zone \(project-id/zone-name\) and might be in use by another cluster, please remove it to continue$`, + }, { + name: "pre-existing", + records: []*dns.ResourceRecordSet{{Name: "api.cluster-name.base-domain."}, {Name: "api.cluster-name.base-domain."}}, + err: `^metadata\.name: Invalid value: "cluster-name": record api\.cluster-name\.base-domain\. already exists in DNS Zone \(project-id/zone-name\) and might be in use by another cluster, please remove it to continue$`, + }} + + for _, test := range cases { + t.Run(test.name, func(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + gcpClient := mock.NewMockAPI(mockCtrl) + + gcpClient.EXPECT().GetPublicDNSZone(gomock.Any(), "base-domain", "project-id").Return(&dns.ManagedZone{Name: "zone-name"}, nil).AnyTimes() + gcpClient.EXPECT().GetRecordSets(gomock.Any(), gomock.Eq("project-id"), gomock.Eq("zone-name")).Return(test.records, nil).AnyTimes() + + err := ValidatePreExitingPublicDNS(gcpClient, &types.InstallConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster-name"}, + BaseDomain: "base-domain", + Platform: types.Platform{GCP: &gcp.Platform{ProjectID: "project-id"}}, + }) + if test.err == "" { + assert.NoError(t, err) + } else { + assert.Regexp(t, test.err, err) + } + }) + } +} From ed69403157a2d2c78283a61cabe4273ef6f2fd1c Mon Sep 17 00:00:00 2001 From: Abhinav Dahiya Date: Wed, 13 May 2020 21:23:26 -0700 Subject: [PATCH 3/4] asset/installconfig/platformprovisioncheck.go: check for api dns record before creating cluster --- pkg/asset/installconfig/platformprovisioncheck.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/pkg/asset/installconfig/platformprovisioncheck.go b/pkg/asset/installconfig/platformprovisioncheck.go index 96522c92ad5..1639fe81372 100644 --- a/pkg/asset/installconfig/platformprovisioncheck.go +++ b/pkg/asset/installconfig/platformprovisioncheck.go @@ -1,10 +1,12 @@ package installconfig import ( + "context" "fmt" "github.com/openshift/installer/pkg/asset" azconfig "github.com/openshift/installer/pkg/asset/installconfig/azure" + gcpconfig "github.com/openshift/installer/pkg/asset/installconfig/gcp" vsconfig "github.com/openshift/installer/pkg/asset/installconfig/vsphere" "github.com/openshift/installer/pkg/types/aws" "github.com/openshift/installer/pkg/types/azure" @@ -49,7 +51,16 @@ func (a *PlatformProvisionCheck) Generate(dependencies asset.Parents) error { if err != nil { return err } - case aws.Name, baremetal.Name, gcp.Name, libvirt.Name, none.Name, openstack.Name, ovirt.Name: + case gcp.Name: + client, err := gcpconfig.NewClient(context.TODO()) + if err != nil { + return err + } + err = gcpconfig.ValidatePreExitingPublicDNS(client, ic.Config) + if err != nil { + return err + } + case aws.Name, baremetal.Name, libvirt.Name, none.Name, openstack.Name, ovirt.Name: // no special provisioning requirements to check default: err = fmt.Errorf("unknown platform type %q", platform) From 52586ea5810b0e8b884dc90f6c35beb3e2e95ccc Mon Sep 17 00:00:00 2001 From: Abhinav Dahiya Date: Thu, 14 May 2020 11:46:08 -0700 Subject: [PATCH 4/4] installconfig/gcp: fix the GetPublicZone api definition to match the actual impl --- pkg/asset/installconfig/gcp/client.go | 6 ++++-- pkg/asset/installconfig/gcp/dns.go | 6 ------ pkg/asset/installconfig/gcp/validation.go | 2 +- pkg/asset/installconfig/gcp/validation_test.go | 2 +- 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/pkg/asset/installconfig/gcp/client.go b/pkg/asset/installconfig/gcp/client.go index 937c1632297..61d7cd02b75 100644 --- a/pkg/asset/installconfig/gcp/client.go +++ b/pkg/asset/installconfig/gcp/client.go @@ -19,7 +19,7 @@ import ( type API interface { GetNetwork(ctx context.Context, network, project string) (*compute.Network, error) GetPublicDomains(ctx context.Context, project string) ([]string, error) - GetPublicDNSZone(ctx context.Context, baseDomain, project string) (*dns.ManagedZone, error) + GetPublicDNSZone(ctx context.Context, project, baseDomain string) (*dns.ManagedZone, error) GetSubnetworks(ctx context.Context, network, project, region string) ([]*compute.Subnetwork, error) GetProjects(ctx context.Context) (map[string]string, error) GetRecordSets(ctx context.Context, project, zone string) ([]*dns.ResourceRecordSet, error) @@ -96,7 +96,9 @@ func (c *Client) GetPublicDNSZone(ctx context.Context, project, baseDomain strin if err != nil { return nil, err } - + if !strings.HasSuffix(baseDomain, ".") { + baseDomain = fmt.Sprintf("%s.", baseDomain) + } req := svc.ManagedZones.List(project).DnsName(baseDomain).Context(ctx) var res *dns.ManagedZone if err := req.Pages(ctx, func(page *dns.ManagedZonesListResponse) error { diff --git a/pkg/asset/installconfig/gcp/dns.go b/pkg/asset/installconfig/gcp/dns.go index 590c86ab9c3..06f44602931 100644 --- a/pkg/asset/installconfig/gcp/dns.go +++ b/pkg/asset/installconfig/gcp/dns.go @@ -2,9 +2,7 @@ package gcp import ( "context" - "fmt" "sort" - "strings" "time" "github.com/pkg/errors" @@ -23,10 +21,6 @@ func GetPublicZone(ctx context.Context, project, baseDomain string) (*dns.Manage ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute) defer cancel() - if !strings.HasSuffix(baseDomain, ".") { - baseDomain = fmt.Sprintf("%s.", baseDomain) - } - dnsZone, err := client.GetPublicDNSZone(ctx, project, baseDomain) if err != nil { return nil, err diff --git a/pkg/asset/installconfig/gcp/validation.go b/pkg/asset/installconfig/gcp/validation.go index 163fb895ab9..f441f14ddcc 100644 --- a/pkg/asset/installconfig/gcp/validation.go +++ b/pkg/asset/installconfig/gcp/validation.go @@ -35,7 +35,7 @@ func ValidatePreExitingPublicDNS(client API, ic *types.InstallConfig) error { record := fmt.Sprintf("api.%s.", strings.TrimSuffix(ic.ClusterDomain(), ".")) - zone, err := client.GetPublicDNSZone(context.TODO(), ic.BaseDomain, ic.Platform.GCP.ProjectID) + zone, err := client.GetPublicDNSZone(context.TODO(), ic.Platform.GCP.ProjectID, ic.BaseDomain) if err != nil { var gErr *googleapi.Error if errors.As(err, &gErr) { diff --git a/pkg/asset/installconfig/gcp/validation_test.go b/pkg/asset/installconfig/gcp/validation_test.go index ecc81e07338..f390d81679c 100644 --- a/pkg/asset/installconfig/gcp/validation_test.go +++ b/pkg/asset/installconfig/gcp/validation_test.go @@ -213,7 +213,7 @@ func TestValidatePreExitingPublicDNS(t *testing.T) { defer mockCtrl.Finish() gcpClient := mock.NewMockAPI(mockCtrl) - gcpClient.EXPECT().GetPublicDNSZone(gomock.Any(), "base-domain", "project-id").Return(&dns.ManagedZone{Name: "zone-name"}, nil).AnyTimes() + gcpClient.EXPECT().GetPublicDNSZone(gomock.Any(), "project-id", "base-domain").Return(&dns.ManagedZone{Name: "zone-name"}, nil).AnyTimes() gcpClient.EXPECT().GetRecordSets(gomock.Any(), gomock.Eq("project-id"), gomock.Eq("zone-name")).Return(test.records, nil).AnyTimes() err := ValidatePreExitingPublicDNS(gcpClient, &types.InstallConfig{