diff --git a/pkg/controller/privatelink/actuator/awsactuator/awshubactuator.go b/pkg/controller/privatelink/actuator/awsactuator/awshubactuator.go index 16e31fc352..866f60a75a 100644 --- a/pkg/controller/privatelink/actuator/awsactuator/awshubactuator.go +++ b/pkg/controller/privatelink/actuator/awsactuator/awshubactuator.go @@ -140,6 +140,7 @@ func (a *AWSHubActuator) Reconcile(cd *hivev1.ClusterDeployment, metadata *hivev if err != nil { return reconcile.Result{}, errors.Wrap(err, "failed to update condition on cluster deployment") } + return reconcile.Result{Requeue: true}, nil } logger.Debug("reconciling Hosted Zone Records") @@ -158,6 +159,7 @@ func (a *AWSHubActuator) Reconcile(cd *hivev1.ClusterDeployment, metadata *hivev if err != nil { return reconcile.Result{}, errors.Wrap(err, "failed to update condition on cluster deployment") } + return reconcile.Result{Requeue: true}, nil } logger.Debug("reconciling Hosted Zone Associations") @@ -177,6 +179,7 @@ func (a *AWSHubActuator) Reconcile(cd *hivev1.ClusterDeployment, metadata *hivev if err != nil { return reconcile.Result{}, errors.Wrap(err, "failed to update condition on cluster deployment") } + return reconcile.Result{Requeue: true}, nil } return reconcile.Result{}, nil @@ -374,11 +377,112 @@ func (a *AWSHubActuator) cleanupHostedZone(cd *hivev1.ClusterDeployment, metadat } func (a *AWSHubActuator) ReconcileHostedZoneRecords(cd *hivev1.ClusterDeployment, hostedZoneID string, dnsRecord *actuator.DnsRecord, apiDomain string, logger log.FieldLogger) (bool, error) { + hzLog := logger.WithField("hostedZoneID", hostedZoneID) + modified := false + rSet, err := a.recordSet(cd, apiDomain, dnsRecord) if err != nil { return false, errors.Wrap(err, "error generating DNS records") } + recordsResp, err := a.awsClientHub.ListResourceRecordSets(&route53.ListResourceRecordSetsInput{ + HostedZoneId: aws.String(hostedZoneID), + }) + if err != nil { + return false, errors.Wrapf(err, "failed to list the hosted zone %s", hostedZoneID) + } + + for _, record := range recordsResp.ResourceRecordSets { + if *record.Name == *rSet.Name { + if rSet.ResourceRecords != nil { + if aws.StringValue(record.Type) != aws.StringValue(rSet.Type) { + modified = true + hzLog.WithFields(log.Fields{ + "record": aws.StringValue(rSet.Name), + "type": aws.StringValue(rSet.Type), + }).Debug("updating record type") + } + if aws.Int64Value(record.TTL) != aws.Int64Value(rSet.TTL) { + modified = true + hzLog.WithFields(log.Fields{ + "record": aws.StringValue(rSet.Name), + "ttl": aws.Int64Value(rSet.TTL), + }).Debug("updating record ttl") + } + + oldRecords := sets.NewString() + for _, record := range record.ResourceRecords { + oldRecords.Insert(aws.StringValue(record.Value)) + } + + desiredRecords := sets.NewString() + for _, record := range rSet.ResourceRecords { + desiredRecords.Insert(aws.StringValue(record.Value)) + } + + added := desiredRecords.Difference(oldRecords).List() + removed := oldRecords.Difference(desiredRecords).List() + + if len(added) > 0 || len(removed) > 0 { + modified = true + hzLog.WithFields(log.Fields{ + "added": added, + "removed": removed, + }).Debug("updating the addresses assigned to the dns record") + } + + if !modified { + return false, nil + } + } else if rSet.AliasTarget != nil { + logger.Debugf("AliasTarget") + if record.AliasTarget == nil { + modified = true + hzLog.WithFields(log.Fields{ + "record": aws.StringValue(rSet.Name), + }).Debug("updating the record to use alias target") + break + } + + if aws.StringValue(record.Type) != aws.StringValue(rSet.Type) { + modified = true + hzLog.WithFields(log.Fields{ + "record": aws.StringValue(rSet.Name), + "type": aws.StringValue(rSet.Type), + }).Debug("updating record type") + } + + if aws.StringValue(record.AliasTarget.DNSName) != aws.StringValue(rSet.AliasTarget.DNSName) { + modified = true + hzLog.WithFields(log.Fields{ + "record": aws.StringValue(rSet.Name), + "dnsName": aws.StringValue(rSet.AliasTarget.DNSName), + }).Debug("updating the aliasTarget dnsName") + } + + if aws.StringValue(record.AliasTarget.HostedZoneId) != aws.StringValue(rSet.AliasTarget.HostedZoneId) { + modified = true + hzLog.WithFields(log.Fields{ + "record": aws.StringValue(rSet.Name), + "hostedZoneId": aws.StringValue(rSet.AliasTarget.HostedZoneId), + }).Debug("updating the aliasTarget hostedZoneId") + } + + if aws.BoolValue(record.AliasTarget.EvaluateTargetHealth) != aws.BoolValue(rSet.AliasTarget.EvaluateTargetHealth) { + modified = true + hzLog.WithFields(log.Fields{ + "record": aws.StringValue(rSet.Name), + "evaluateTargetHealth": aws.BoolValue(rSet.AliasTarget.EvaluateTargetHealth), + }).Debug("updating the aliasTarget evaluateTargetHealth") + } + + if !modified { + return false, nil + } + } + break + } + } _, err = a.awsClientHub.ChangeResourceRecordSets(&route53.ChangeResourceRecordSetsInput{ HostedZoneId: aws.String(hostedZoneID), ChangeBatch: &route53.ChangeBatch{ diff --git a/pkg/controller/privatelink/actuator/awsactuator/awshubactuator_test.go b/pkg/controller/privatelink/actuator/awsactuator/awshubactuator_test.go index a396ce5673..8a6a1adb26 100644 --- a/pkg/controller/privatelink/actuator/awsactuator/awshubactuator_test.go +++ b/pkg/controller/privatelink/actuator/awsactuator/awshubactuator_test.go @@ -19,6 +19,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" hivev1 "github.com/openshift/hive/apis/hive/v1" hivev1aws "github.com/openshift/hive/apis/hive/v1/aws" @@ -248,9 +249,10 @@ func Test_Reconcile(t *testing.T) { AWSClientConfig func(*mock.MockClient) + expect reconcile.Result expectConditions []hivev1.ClusterDeploymentCondition expectError string - expectLogs []string + expectStatus *hivev1aws.PrivateLinkAccessStatus }{{ // There should be an error on failure to initialURL name: "failure on initialURL", cd: cdBuilder.Build( @@ -267,7 +269,7 @@ func Test_Reconcile(t *testing.T) { }}, expectError: "could not get API URL from kubeconfig: failed to load the kubeconfig: kubeconfig secret does not contain necessary data", }, { // There should be an error on failure to ensureHostedZone - name: "failure on ensureHostedZone SetErrConditionWithRetry", + name: "failure on ensureHostedZone", cd: cdBuilder.Build( testcd.WithClusterMetadata(&hivev1.ClusterMetadata{AdminKubeconfigSecretRef: corev1.LocalObjectReference{Name: "kubeconfig"}}), ), @@ -282,6 +284,30 @@ func Test_Reconcile(t *testing.T) { Message: "at least one associated VPC must be configured", }}, expectError: "failed to reconcile the Hosted Zone: at least one associated VPC must be configured", + }, { // Ready condition should be updated when hzModified + name: "hzModified", + cd: cdBuilder.Build( + testcd.WithClusterMetadata(&hivev1.ClusterMetadata{AdminKubeconfigSecretRef: corev1.LocalObjectReference{Name: "kubeconfig"}}), + ), + existing: []runtime.Object{ + mockSecret("kubeconfig", mockKubeconfigData), + }, + AWSClientConfig: func(m *mock.MockClient) { + m.EXPECT().ListHostedZonesByVPC(gomock.Any()).Return(&route53.ListHostedZonesByVPCOutput{ + HostedZoneSummaries: []*route53.HostedZoneSummary{{ + Name: aws.String(testAPIDomain), + HostedZoneId: aws.String(testHostedZone), + }}, + }, nil) + }, + expect: reconcile.Result{Requeue: true}, + expectConditions: []hivev1.ClusterDeploymentCondition{{ + Status: corev1.ConditionFalse, + Type: hivev1.PrivateLinkReadyClusterDeploymentCondition, + Reason: "ReconciledPrivateHostedZone", + Message: "reconciled the Private Hosted Zone for the VPC Endpoint of the cluster", + }}, + expectStatus: &hivev1aws.PrivateLinkAccessStatus{HostedZoneID: testHostedZone}, }, { // There should be an error on failure to ReconcileHostedZoneRecords name: "failure on ReconcileHostedZoneRecords", cd: cdBuilder.Build( @@ -310,6 +336,7 @@ func Test_Reconcile(t *testing.T) { Message: "error generating DNS records: configured to use ip address, but no address found.", }}, expectError: "failed to reconcile the Hosted Zone Records: error generating DNS records: configured to use ip address, but no address found.", + // }, { // Ready condition should be updated when recordsModified }, { // There should be an error on failure to reconcileHostedZoneAssociations name: "failure on reconcileHostedZoneAssociations", cd: cdBuilder.Build( @@ -331,7 +358,16 @@ func Test_Reconcile(t *testing.T) { HostedZoneId: aws.String(testHostedZone), }}, }, nil) - m.EXPECT().ChangeResourceRecordSets(gomock.Any()).Return(nil, nil) + m.EXPECT().ListResourceRecordSets(gomock.Any()).Return(&route53.ListResourceRecordSetsOutput{ + ResourceRecordSets: []*route53.ResourceRecordSet{{ + Name: aws.String(testAPIDomain), + ResourceRecords: []*route53.ResourceRecord{{ + Value: aws.String("0.0.0.1"), + }}, + Type: aws.String("A"), + TTL: aws.Int64(10), + }}, + }, nil) m.EXPECT().GetHostedZone(gomock.Any()).Return(nil, awserr.New("AccessDenied", "not authorized to GetHostedZone", nil)) }, expectConditions: []hivev1.ClusterDeploymentCondition{{ @@ -341,7 +377,52 @@ func Test_Reconcile(t *testing.T) { Message: "failed to get the Hosted Zone: AccessDenied: not authorized to GetHostedZone", }}, expectError: "failed to reconcile the Hosted Zone Associations: failed to get the Hosted Zone: AccessDenied: not authorized to GetHostedZone", - }, { // There should be no errors on success + }, { // Ready condition should be updated on associationsModified + name: "associationsModified", + cd: cdBuilder.Build( + testcd.WithClusterMetadata(&hivev1.ClusterMetadata{AdminKubeconfigSecretRef: corev1.LocalObjectReference{Name: "kubeconfig"}}), + testcd.WithAWSPlatformStatus(&hivev1aws.PlatformStatus{ + PrivateLink: &hivev1aws.PrivateLinkAccessStatus{ + HostedZoneID: testHostedZone, + }, + }), + ), + existing: []runtime.Object{ + mockSecret("kubeconfig", mockKubeconfigData), + }, + record: &actuator.DnsRecord{IpAddress: []string{"0.0.0.1"}}, + AWSClientConfig: func(m *mock.MockClient) { + m.EXPECT().ListHostedZonesByVPC(gomock.Any()).Return(&route53.ListHostedZonesByVPCOutput{ + HostedZoneSummaries: []*route53.HostedZoneSummary{{ + Name: aws.String(testAPIDomain), + HostedZoneId: aws.String(testHostedZone), + }}, + }, nil) + m.EXPECT().ListResourceRecordSets(gomock.Any()).Return(&route53.ListResourceRecordSetsOutput{ + ResourceRecordSets: []*route53.ResourceRecordSet{{ + Name: aws.String(testAPIDomain), + ResourceRecords: []*route53.ResourceRecord{{ + Value: aws.String("0.0.0.1"), + }}, + Type: aws.String("A"), + TTL: aws.Int64(10), + }}, + }, nil) + m.EXPECT().GetHostedZone(gomock.Any()).Return(&route53.GetHostedZoneOutput{ + HostedZone: &route53.HostedZone{ + Id: aws.String(testHostedZone), + }, + }, nil) + m.EXPECT().AssociateVPCWithHostedZone(gomock.Any()).Return(&route53.AssociateVPCWithHostedZoneOutput{}, nil) + }, + expect: reconcile.Result{Requeue: true}, + expectConditions: []hivev1.ClusterDeploymentCondition{{ + Status: corev1.ConditionFalse, + Type: hivev1.PrivateLinkReadyClusterDeploymentCondition, + Reason: "ReconciledAssociationsToVPCs", + Message: "reconciled the associations of all the required VPCs to the Private Hosted Zone for the VPC Endpoint", + }}, + }, { // There should be no error on success name: "success", cd: cdBuilder.Build( testcd.WithClusterMetadata(&hivev1.ClusterMetadata{AdminKubeconfigSecretRef: corev1.LocalObjectReference{Name: "kubeconfig"}}), @@ -362,7 +443,16 @@ func Test_Reconcile(t *testing.T) { HostedZoneId: aws.String(testHostedZone), }}, }, nil) - m.EXPECT().ChangeResourceRecordSets(gomock.Any()).Return(nil, nil) + m.EXPECT().ListResourceRecordSets(gomock.Any()).Return(&route53.ListResourceRecordSetsOutput{ + ResourceRecordSets: []*route53.ResourceRecordSet{{ + Name: aws.String(testAPIDomain), + ResourceRecords: []*route53.ResourceRecord{{ + Value: aws.String("0.0.0.1"), + }}, + Type: aws.String("A"), + TTL: aws.Int64(10), + }}, + }, nil) m.EXPECT().GetHostedZone(gomock.Any()).Return(&route53.GetHostedZoneOutput{ HostedZone: &route53.HostedZone{ Id: aws.String(testHostedZone), @@ -380,7 +470,7 @@ func Test_Reconcile(t *testing.T) { if test.config == nil { test.config = &hivev1.AWSPrivateLinkConfig{AssociatedVPCs: mockAssociatedVPCs} } - logger, hook := testlogger.NewLoggerWithHook() + logger, _ := testlogger.NewLoggerWithHook() awsHubActuator, err := newTestAWSHubActuator(t, test.config, test.cd, @@ -398,15 +488,15 @@ func Test_Reconcile(t *testing.T) { assert.EqualError(t, err, test.expectError) } + if test.expectStatus != nil { + assert.Equal(t, test.expectStatus, test.cd.Status.Platform.AWS.PrivateLink) + } + curr := &hivev1.ClusterDeployment{} fakeClient := *awsHubActuator.client errGet := fakeClient.Get(context.TODO(), types.NamespacedName{Namespace: test.cd.Namespace, Name: test.cd.Name}, curr) assert.NoError(t, errGet) testassert.AssertConditions(t, curr, test.expectConditions) - - for _, log := range test.expectLogs { - testlogger.AssertHookContainsMessage(t, hook, log) - } }) } } @@ -978,11 +1068,239 @@ func Test_ReconcileHostedZoneRecords(t *testing.T) { name: "recordSet failed", cd: cdBuilder.Build(), expectError: "error generating DNS records: configured to use ip address, but no address found.", + }, { // There should be an error on ListResourceRecordSets failure + name: "ListResourceRecordSets failed", + cd: cdBuilder.Build(), + record: &actuator.DnsRecord{IpAddress: []string{"0.0.0.1"}}, + AWSClientConfig: func(m *mock.MockClient) { + m.EXPECT().ListResourceRecordSets(gomock.Any()).Return(nil, awserr.New("AccessDenied", "not authorized to ListResourceRecordSets", nil)) + }, + expectError: "failed to list the hosted zone testhostedzone: AccessDenied: not authorized to ListResourceRecordSets", + }, { // Should update record Type when using ResourceRecords and Type is different + name: "ResourceRecords type is different", + cd: cdBuilder.Build(), + record: &actuator.DnsRecord{IpAddress: []string{"0.0.0.1", "0.0.0.2"}}, + AWSClientConfig: func(m *mock.MockClient) { + m.EXPECT().ListResourceRecordSets(gomock.Any()).Return(&route53.ListResourceRecordSetsOutput{ + ResourceRecordSets: []*route53.ResourceRecordSet{{ + Name: aws.String(testAPIDomain), + ResourceRecords: []*route53.ResourceRecord{{ + Value: aws.String("0.0.0.1"), + }, { + Value: aws.String("0.0.0.2"), + }}, + Type: aws.String("different"), + TTL: aws.Int64(10), + }}, + }, nil) + m.EXPECT().ChangeResourceRecordSets(gomock.Any()).Return(nil, nil) + }, + expect: true, + }, { // Should update record TTL when using ResourceRecords and TTL is different + name: "ResourceRecords ttl is different", + cd: cdBuilder.Build(), + record: &actuator.DnsRecord{IpAddress: []string{"0.0.0.1", "0.0.0.2"}}, + AWSClientConfig: func(m *mock.MockClient) { + m.EXPECT().ListResourceRecordSets(gomock.Any()).Return(&route53.ListResourceRecordSetsOutput{ + ResourceRecordSets: []*route53.ResourceRecordSet{{ + Name: aws.String(testAPIDomain), + ResourceRecords: []*route53.ResourceRecord{{ + Value: aws.String("0.0.0.1"), + }, { + Value: aws.String("0.0.0.2"), + }}, + Type: aws.String("A"), + TTL: aws.Int64(11), + }}, + }, nil) + m.EXPECT().ChangeResourceRecordSets(gomock.Any()).Return(nil, nil) + }, + expect: true, + }, { // Should update records when using ResourceRecords and an address is missing + name: "ResourceRecords missing address", + cd: cdBuilder.Build(), + record: &actuator.DnsRecord{IpAddress: []string{"0.0.0.1", "0.0.0.2"}}, + AWSClientConfig: func(m *mock.MockClient) { + m.EXPECT().ListResourceRecordSets(gomock.Any()).Return(&route53.ListResourceRecordSetsOutput{ + ResourceRecordSets: []*route53.ResourceRecordSet{{ + Name: aws.String(testAPIDomain), + ResourceRecords: []*route53.ResourceRecord{{ + Value: aws.String("0.0.0.1"), + }}, + Type: aws.String("A"), + TTL: aws.Int64(10), + }}, + }, nil) + m.EXPECT().ChangeResourceRecordSets(gomock.Any()).Return(nil, nil) + }, + expect: true, + }, { // Should update records when using ResourceRecords and there is an extra address + name: "ResourceRecords missing address", + cd: cdBuilder.Build(), + record: &actuator.DnsRecord{IpAddress: []string{"0.0.0.1", "0.0.0.2"}}, + AWSClientConfig: func(m *mock.MockClient) { + m.EXPECT().ListResourceRecordSets(gomock.Any()).Return(&route53.ListResourceRecordSetsOutput{ + ResourceRecordSets: []*route53.ResourceRecordSet{{ + Name: aws.String(testAPIDomain), + ResourceRecords: []*route53.ResourceRecord{{ + Value: aws.String("0.0.0.1"), + }, { + Value: aws.String("0.0.0.2"), + }, { + Value: aws.String("0.0.0.3"), + }}, + Type: aws.String("A"), + TTL: aws.Int64(10), + }}, + }, nil) + m.EXPECT().ChangeResourceRecordSets(gomock.Any()).Return(nil, nil) + }, + expect: true, + }, { // No changes when using ResourceRecords and everything matches + name: "ResourceRecords no change", + cd: cdBuilder.Build(), + record: &actuator.DnsRecord{IpAddress: []string{"0.0.0.1", "0.0.0.2"}}, + AWSClientConfig: func(m *mock.MockClient) { + m.EXPECT().ListResourceRecordSets(gomock.Any()).Return(&route53.ListResourceRecordSetsOutput{ + ResourceRecordSets: []*route53.ResourceRecordSet{{ + Name: aws.String(testAPIDomain), + ResourceRecords: []*route53.ResourceRecord{{ + Value: aws.String("0.0.0.1"), + }, { + Value: aws.String("0.0.0.2"), + }}, + Type: aws.String("A"), + TTL: aws.Int64(10), + }}, + }, nil) + }, + }, { // Should update record when using AliasTarget and AliasTarget is nil + name: "AliasTarget is nil", + cd: cdBuilder.Build(testcd.WithAWSPlatform(&hivev1aws.Platform{})), + record: &actuator.DnsRecord{AliasTarget: actuator.AliasTarget{ + Name: "testAliasTarget", + HostedZoneID: testHostedZone, + }}, + AWSClientConfig: func(m *mock.MockClient) { + m.EXPECT().ListResourceRecordSets(gomock.Any()).Return(&route53.ListResourceRecordSetsOutput{ + ResourceRecordSets: []*route53.ResourceRecordSet{{ + Name: aws.String(testAPIDomain), + Type: aws.String("A"), + }}, + }, nil) + m.EXPECT().ChangeResourceRecordSets(gomock.Any()).Return(nil, nil) + }, + expect: true, + }, { // Should update record Type when using AliasTarget and Type is different + name: "AliasTarget type is different", + cd: cdBuilder.Build(testcd.WithAWSPlatform(&hivev1aws.Platform{})), + record: &actuator.DnsRecord{AliasTarget: actuator.AliasTarget{ + Name: "testAliasTarget", + HostedZoneID: testHostedZone, + }}, + AWSClientConfig: func(m *mock.MockClient) { + m.EXPECT().ListResourceRecordSets(gomock.Any()).Return(&route53.ListResourceRecordSetsOutput{ + ResourceRecordSets: []*route53.ResourceRecordSet{{ + Name: aws.String(testAPIDomain), + AliasTarget: &route53.AliasTarget{ + DNSName: aws.String("testAliasTarget"), + HostedZoneId: aws.String(testHostedZone), + }, + Type: aws.String("different"), + }}, + }, nil) + m.EXPECT().ChangeResourceRecordSets(gomock.Any()).Return(nil, nil) + }, + expect: true, + }, { // Should update record DNSName when using AliasTarget and DNSName is different + name: "DNSName type is different", + cd: cdBuilder.Build(testcd.WithAWSPlatform(&hivev1aws.Platform{})), + record: &actuator.DnsRecord{AliasTarget: actuator.AliasTarget{ + Name: "testAliasTarget", + HostedZoneID: testHostedZone, + }}, + AWSClientConfig: func(m *mock.MockClient) { + m.EXPECT().ListResourceRecordSets(gomock.Any()).Return(&route53.ListResourceRecordSetsOutput{ + ResourceRecordSets: []*route53.ResourceRecordSet{{ + Name: aws.String(testAPIDomain), + AliasTarget: &route53.AliasTarget{ + DNSName: aws.String("different"), + HostedZoneId: aws.String(testHostedZone), + }, + Type: aws.String("A"), + }}, + }, nil) + m.EXPECT().ChangeResourceRecordSets(gomock.Any()).Return(nil, nil) + }, + expect: true, + }, { // Should update record HostedZoneId when using AliasTarget and HostedZoneId is different + name: "HostedZoneId type is different", + cd: cdBuilder.Build(testcd.WithAWSPlatform(&hivev1aws.Platform{})), + record: &actuator.DnsRecord{AliasTarget: actuator.AliasTarget{ + Name: "testAliasTarget", + HostedZoneID: testHostedZone, + }}, + AWSClientConfig: func(m *mock.MockClient) { + m.EXPECT().ListResourceRecordSets(gomock.Any()).Return(&route53.ListResourceRecordSetsOutput{ + ResourceRecordSets: []*route53.ResourceRecordSet{{ + Name: aws.String(testAPIDomain), + AliasTarget: &route53.AliasTarget{ + DNSName: aws.String("testAliasTarget"), + HostedZoneId: aws.String("different"), + }, + Type: aws.String("A"), + }}, + }, nil) + m.EXPECT().ChangeResourceRecordSets(gomock.Any()).Return(nil, nil) + }, + expect: true, + }, { // Should update record EvaluateTargetHealth when using AliasTarget and EvaluateTargetHealth is different + name: "EvaluateTargetHealth type is different", + cd: cdBuilder.Build(testcd.WithAWSPlatform(&hivev1aws.Platform{})), + record: &actuator.DnsRecord{AliasTarget: actuator.AliasTarget{ + Name: "testAliasTarget", + HostedZoneID: testHostedZone, + }}, + AWSClientConfig: func(m *mock.MockClient) { + m.EXPECT().ListResourceRecordSets(gomock.Any()).Return(&route53.ListResourceRecordSetsOutput{ + ResourceRecordSets: []*route53.ResourceRecordSet{{ + Name: aws.String(testAPIDomain), + AliasTarget: &route53.AliasTarget{ + DNSName: aws.String("testAliasTarget"), + HostedZoneId: aws.String(testHostedZone), + EvaluateTargetHealth: aws.Bool(true), + }, + Type: aws.String("A"), + }}, + }, nil) + m.EXPECT().ChangeResourceRecordSets(gomock.Any()).Return(nil, nil) + }, + expect: true, + }, { // No changes when using ResourceRecords and everything matches + name: "AliasTarget no change", + cd: cdBuilder.Build(testcd.WithAWSPlatform(&hivev1aws.Platform{})), + record: &actuator.DnsRecord{AliasTarget: actuator.AliasTarget{ + Name: "testAliasTarget", + HostedZoneID: testHostedZone, + }}, + AWSClientConfig: func(m *mock.MockClient) { + m.EXPECT().ListResourceRecordSets(gomock.Any()).Return(&route53.ListResourceRecordSetsOutput{ + ResourceRecordSets: []*route53.ResourceRecordSet{{ + Name: aws.String(testAPIDomain), + AliasTarget: &route53.AliasTarget{ + DNSName: aws.String("testAliasTarget"), + HostedZoneId: aws.String(testHostedZone), + }, + Type: aws.String("A"), + }}, + }, nil) + }, }, { // There should be an error on failure to change the record set in the aws api name: "ChangeResourceRecordSets failed", cd: cdBuilder.Build(), record: &actuator.DnsRecord{IpAddress: []string{"0.0.0.1"}}, AWSClientConfig: func(m *mock.MockClient) { + m.EXPECT().ListResourceRecordSets(gomock.Any()).Return(&route53.ListResourceRecordSetsOutput{}, nil) m.EXPECT().ChangeResourceRecordSets(gomock.Any()).Return(nil, awserr.New("AccessDenied", "not authorized to ChangeResourceRecordSets", nil)) }, expectError: "error adding record to Hosted Zone testhostedzone for VPC Endpoint: AccessDenied: not authorized to ChangeResourceRecordSets", @@ -991,6 +1309,7 @@ func Test_ReconcileHostedZoneRecords(t *testing.T) { cd: cdBuilder.Build(), record: &actuator.DnsRecord{IpAddress: []string{"0.0.0.1"}}, AWSClientConfig: func(m *mock.MockClient) { + m.EXPECT().ListResourceRecordSets(gomock.Any()).Return(&route53.ListResourceRecordSetsOutput{}, nil) m.EXPECT().ChangeResourceRecordSets(&route53.ChangeResourceRecordSetsInput{ HostedZoneId: aws.String(testHostedZone), ChangeBatch: &route53.ChangeBatch{ diff --git a/pkg/controller/privatelink/actuator/gcpactuator/gcplinkactuator.go b/pkg/controller/privatelink/actuator/gcpactuator/gcplinkactuator.go index 18dd246cee..fa254f41e2 100644 --- a/pkg/controller/privatelink/actuator/gcpactuator/gcplinkactuator.go +++ b/pkg/controller/privatelink/actuator/gcpactuator/gcplinkactuator.go @@ -179,6 +179,7 @@ func (a *GCPLinkActuator) Reconcile(cd *hivev1.ClusterDeployment, metadata *hive if err != nil { return reconcile.Result{}, errors.Wrap(err, "failed to update condition on cluster deployment") } + return reconcile.Result{Requeue: true}, nil } subnet = newSubnet @@ -198,6 +199,7 @@ func (a *GCPLinkActuator) Reconcile(cd *hivev1.ClusterDeployment, metadata *hive if err != nil { return reconcile.Result{}, errors.Wrap(err, "failed to update condition on cluster deployment") } + return reconcile.Result{Requeue: true}, nil } } @@ -217,6 +219,7 @@ func (a *GCPLinkActuator) Reconcile(cd *hivev1.ClusterDeployment, metadata *hive if err != nil { return reconcile.Result{}, errors.Wrap(err, "failed to update condition on cluster deployment") } + return reconcile.Result{Requeue: true}, nil } logger.Debug("reconciling Endpoint Address") @@ -235,6 +238,7 @@ func (a *GCPLinkActuator) Reconcile(cd *hivev1.ClusterDeployment, metadata *hive if err != nil { return reconcile.Result{}, errors.Wrap(err, "failed to update condition on cluster deployment") } + return reconcile.Result{Requeue: true}, nil } logger.Debug("reconciling Endpoint") @@ -253,6 +257,7 @@ func (a *GCPLinkActuator) Reconcile(cd *hivev1.ClusterDeployment, metadata *hive if err != nil { return reconcile.Result{}, errors.Wrap(err, "failed to update condition on cluster deployment") } + return reconcile.Result{Requeue: true}, nil } // Set the DNS IP Addresses for the hub actuator. diff --git a/pkg/controller/privatelink/actuator/gcpactuator/gcplinkactuator_test.go b/pkg/controller/privatelink/actuator/gcpactuator/gcplinkactuator_test.go index e4f5851822..5265bb86ba 100644 --- a/pkg/controller/privatelink/actuator/gcpactuator/gcplinkactuator_test.go +++ b/pkg/controller/privatelink/actuator/gcpactuator/gcplinkactuator_test.go @@ -447,9 +447,9 @@ func Test_Reconcile(t *testing.T) { }, { // Should return requeue later when forwarding rule is not found name: "forwarding rule not found", cd: cdBuilder.Build( + testcd.WithClusterMetadata(&hivev1.ClusterMetadata{InfraID: testInfraID}), testcd.WithGCPPlatform(&hivev1gcp.Platform{Region: testRegion}), testcd.WithGCPPlatformStatus(&hivev1gcp.PlatformStatus{PrivateServiceConnect: &hivev1gcp.PrivateServiceConnectStatus{}}), - testcd.WithClusterMetadata(&hivev1.ClusterMetadata{InfraID: testInfraID}), ), config: &hivev1.GCPPrivateServiceConnectConfig{ EndpointVPCInventory: []hivev1.GCPPrivateServiceConnectInventory{{ @@ -471,9 +471,9 @@ func Test_Reconcile(t *testing.T) { }, { // There should be an error on GetForwardingRule failure name: "GetForwardingRule failure", cd: cdBuilder.Build( + testcd.WithClusterMetadata(&hivev1.ClusterMetadata{InfraID: testInfraID}), testcd.WithGCPPlatform(&hivev1gcp.Platform{Region: testRegion}), testcd.WithGCPPlatformStatus(&hivev1gcp.PlatformStatus{PrivateServiceConnect: &hivev1gcp.PrivateServiceConnectStatus{}}), - testcd.WithClusterMetadata(&hivev1.ClusterMetadata{InfraID: testInfraID}), ), config: &hivev1.GCPPrivateServiceConnectConfig{ EndpointVPCInventory: []hivev1.GCPPrivateServiceConnectInventory{{ @@ -495,6 +495,7 @@ func Test_Reconcile(t *testing.T) { }, { // There should be an error on GetSubnet failure when existing subnet is specified name: "Existing Subnet, GetSubnet failure", cd: cdBuilder.Build( + testcd.WithClusterMetadata(&hivev1.ClusterMetadata{InfraID: testInfraID}), testcd.WithGCPPlatform(&hivev1gcp.Platform{ PrivateServiceConnect: &hivev1gcp.PrivateServiceConnect{ ServiceAttachment: &hivev1gcp.ServiceAttachment{ @@ -508,7 +509,6 @@ func Test_Reconcile(t *testing.T) { Region: testRegion, }), testcd.WithGCPPlatformStatus(&hivev1gcp.PlatformStatus{PrivateServiceConnect: &hivev1gcp.PrivateServiceConnectStatus{}}), - testcd.WithClusterMetadata(&hivev1.ClusterMetadata{InfraID: testInfraID}), ), config: &hivev1.GCPPrivateServiceConnectConfig{ EndpointVPCInventory: []hivev1.GCPPrivateServiceConnectInventory{{ @@ -543,7 +543,11 @@ func Test_Reconcile(t *testing.T) { }, Region: testRegion, }), - testcd.WithGCPPlatformStatus(&hivev1gcp.PlatformStatus{PrivateServiceConnect: &hivev1gcp.PrivateServiceConnectStatus{}}), + testcd.WithGCPPlatformStatus(&hivev1gcp.PlatformStatus{PrivateServiceConnect: &hivev1gcp.PrivateServiceConnectStatus{ + ServiceAttachment: mockServiceAttachment.SelfLink, + EndpointAddress: mockAddress.SelfLink, + Endpoint: mockForwardingRule.SelfLink, + }}), testcd.WithClusterMetadata(&hivev1.ClusterMetadata{InfraID: testInfraID}), ), config: &hivev1.GCPPrivateServiceConnectConfig{ @@ -566,12 +570,6 @@ func Test_Reconcile(t *testing.T) { m.EXPECT().GetAddress(gomock.Any(), gomock.Any()).Return(mockAddress, nil) m.EXPECT().GetForwardingRule(gomock.Any(), gomock.Any()).Return(mockForwardingRule, nil) }, - expectConditions: []hivev1.ClusterDeploymentCondition{{ - Status: corev1.ConditionFalse, - Type: hivev1.PrivateLinkReadyClusterDeploymentCondition, - Reason: "ReconciledEndpoint", - Message: "reconciled the Endpoint", - }}, expectStatus: &hivev1gcp.PrivateServiceConnectStatus{ ServiceAttachment: mockServiceAttachment.SelfLink, EndpointAddress: mockAddress.SelfLink, @@ -581,9 +579,9 @@ func Test_Reconcile(t *testing.T) { }, { // There should be an error on GetNetwork failure name: "GetNetwork failure", cd: cdBuilder.Build( + testcd.WithClusterMetadata(&hivev1.ClusterMetadata{InfraID: testInfraID}), testcd.WithGCPPlatform(&hivev1gcp.Platform{Region: testRegion}), testcd.WithGCPPlatformStatus(&hivev1gcp.PlatformStatus{PrivateServiceConnect: &hivev1gcp.PrivateServiceConnectStatus{}}), - testcd.WithClusterMetadata(&hivev1.ClusterMetadata{InfraID: testInfraID}), ), config: &hivev1.GCPPrivateServiceConnectConfig{ EndpointVPCInventory: []hivev1.GCPPrivateServiceConnectInventory{{ @@ -606,9 +604,9 @@ func Test_Reconcile(t *testing.T) { }, { // There should be an error on ensureServiceAttachmentSubnet failure name: "ensureServiceAttachmentSubnet failure", cd: cdBuilder.Build( + testcd.WithClusterMetadata(&hivev1.ClusterMetadata{InfraID: testInfraID}), testcd.WithGCPPlatform(&hivev1gcp.Platform{Region: testRegion}), testcd.WithGCPPlatformStatus(&hivev1gcp.PlatformStatus{PrivateServiceConnect: &hivev1gcp.PrivateServiceConnectStatus{}}), - testcd.WithClusterMetadata(&hivev1.ClusterMetadata{InfraID: testInfraID}), ), config: &hivev1.GCPPrivateServiceConnectConfig{ EndpointVPCInventory: []hivev1.GCPPrivateServiceConnectInventory{{ @@ -635,12 +633,50 @@ func Test_Reconcile(t *testing.T) { Message: "googleapi: Error 401: not authorized to GetSubnet, AccessDenied", }}, expectError: "failed to reconcile the Service Attachment Subnet: googleapi: Error 401: not authorized to GetSubnet, AccessDenied", - }, { // There should be an error on ensureServiceAttachmentFirewall failure - name: "ensureServiceAttachmentFirewall failure", + }, { // Ready condition should be updated when subnetModified + name: "subnetModified", cd: cdBuilder.Build( + testcd.WithClusterMetadata(&hivev1.ClusterMetadata{InfraID: testInfraID}), testcd.WithGCPPlatform(&hivev1gcp.Platform{Region: testRegion}), testcd.WithGCPPlatformStatus(&hivev1gcp.PlatformStatus{PrivateServiceConnect: &hivev1gcp.PrivateServiceConnectStatus{}}), + ), + config: &hivev1.GCPPrivateServiceConnectConfig{ + EndpointVPCInventory: []hivev1.GCPPrivateServiceConnectInventory{{ + Network: mockNetwork.Name, + Subnets: []hivev1.GCPPrivateServiceConnectSubnet{{ + Subnet: mockSubnet.SelfLink, + Region: testRegion, + }}, + }}, + }, + gcpClientConfig: func(m *mockclient.MockClient) { + m.EXPECT().GetSubnet(gomock.Any(), gomock.Any(), gomock.Any()).Return(mockSubnet, nil) + m.EXPECT().ListAddresses(gomock.Any(), gomock.Any()).Return(&compute.AddressList{ + Items: []*compute.Address{mockAddress}, + }, nil) + m.EXPECT().GetForwardingRule(gomock.Any(), gomock.Any()).Return(mockForwardingRule, nil) + m.EXPECT().GetNetwork(gomock.Any()).Return(mockNetwork, nil) + m.EXPECT().GetSubnet(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, newGoogleApiError(http.StatusNotFound, "NotFound", "Subnet not found")) + m.EXPECT().CreateSubnet(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(mockSubnet, nil) + }, + expect: reconcile.Result{Requeue: true}, + expectConditions: []hivev1.ClusterDeploymentCondition{{ + Status: corev1.ConditionFalse, + Type: hivev1.PrivateLinkReadyClusterDeploymentCondition, + Reason: "ReconciledServiceAttachmentSubnet", + Message: "reconciled the Service Attachment Subnet", + }}, + expectStatus: &hivev1gcp.PrivateServiceConnectStatus{ + ServiceAttachmentSubnet: mockSubnet.SelfLink, + }, + }, { // There should be an error on ensureServiceAttachmentFirewall failure + name: "ensureServiceAttachmentFirewall failure", + cd: cdBuilder.Build( testcd.WithClusterMetadata(&hivev1.ClusterMetadata{InfraID: testInfraID}), + testcd.WithGCPPlatform(&hivev1gcp.Platform{Region: testRegion}), + testcd.WithGCPPlatformStatus(&hivev1gcp.PlatformStatus{PrivateServiceConnect: &hivev1gcp.PrivateServiceConnectStatus{ + ServiceAttachmentSubnet: mockSubnet.SelfLink, + }}), ), config: &hivev1.GCPPrivateServiceConnectConfig{ EndpointVPCInventory: []hivev1.GCPPrivateServiceConnectInventory{{ @@ -668,15 +704,55 @@ func Test_Reconcile(t *testing.T) { Message: "googleapi: Error 401: not authorized to GetFirewall, AccessDenied", }}, expectError: "failed to reconcile the Service Attachment Firwall: googleapi: Error 401: not authorized to GetFirewall, AccessDenied", + }, { // Ready condition should be updated when firewallModified + name: "firewallModified", + cd: cdBuilder.Build( + testcd.WithClusterMetadata(&hivev1.ClusterMetadata{InfraID: testInfraID}), + testcd.WithGCPPlatform(&hivev1gcp.Platform{Region: testRegion}), + testcd.WithGCPPlatformStatus(&hivev1gcp.PlatformStatus{PrivateServiceConnect: &hivev1gcp.PrivateServiceConnectStatus{ + ServiceAttachmentSubnet: mockSubnet.SelfLink, + }}), + ), + config: &hivev1.GCPPrivateServiceConnectConfig{ + EndpointVPCInventory: []hivev1.GCPPrivateServiceConnectInventory{{ + Network: mockNetwork.Name, + Subnets: []hivev1.GCPPrivateServiceConnectSubnet{{ + Subnet: mockSubnet.SelfLink, + Region: testRegion, + }}, + }}, + }, + gcpClientConfig: func(m *mockclient.MockClient) { + m.EXPECT().GetSubnet(gomock.Any(), gomock.Any(), gomock.Any()).Return(mockSubnet, nil) + m.EXPECT().ListAddresses(gomock.Any(), gomock.Any()).Return(&compute.AddressList{ + Items: []*compute.Address{mockAddress}, + }, nil) + m.EXPECT().GetForwardingRule(gomock.Any(), gomock.Any()).Return(mockForwardingRule, nil) + m.EXPECT().GetNetwork(gomock.Any()).Return(mockNetwork, nil) + m.EXPECT().GetSubnet(gomock.Any(), gomock.Any(), gomock.Any()).Return(mockSubnet, nil) + m.EXPECT().GetFirewall(gomock.Any()).Return(nil, newGoogleApiError(http.StatusNotFound, "NotFound", "Firewall not found")) + m.EXPECT().CreateFirewall(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(mockFirewall, nil) + }, + expect: reconcile.Result{Requeue: true}, + expectConditions: []hivev1.ClusterDeploymentCondition{{ + Status: corev1.ConditionFalse, + Type: hivev1.PrivateLinkReadyClusterDeploymentCondition, + Reason: "ReconciledServiceAttachmentFirewall", + Message: "reconciled the Service Attachment Firewall", + }}, expectStatus: &hivev1gcp.PrivateServiceConnectStatus{ - ServiceAttachmentSubnet: mockSubnet.SelfLink, + ServiceAttachmentSubnet: mockSubnet.SelfLink, + ServiceAttachmentFirewall: mockFirewall.SelfLink, }, }, { // There should be an error on ensureServiceAttachment failure name: "ensureServiceAttachment failure", cd: cdBuilder.Build( - testcd.WithGCPPlatform(&hivev1gcp.Platform{Region: testRegion}), - testcd.WithGCPPlatformStatus(&hivev1gcp.PlatformStatus{PrivateServiceConnect: &hivev1gcp.PrivateServiceConnectStatus{}}), testcd.WithClusterMetadata(&hivev1.ClusterMetadata{InfraID: testInfraID}), + testcd.WithGCPPlatform(&hivev1gcp.Platform{Region: testRegion}), + testcd.WithGCPPlatformStatus(&hivev1gcp.PlatformStatus{PrivateServiceConnect: &hivev1gcp.PrivateServiceConnectStatus{ + ServiceAttachmentSubnet: mockSubnet.SelfLink, + ServiceAttachmentFirewall: mockFirewall.SelfLink, + }}), ), config: &hivev1.GCPPrivateServiceConnectConfig{ EndpointVPCInventory: []hivev1.GCPPrivateServiceConnectInventory{{ @@ -705,16 +781,60 @@ func Test_Reconcile(t *testing.T) { Message: "googleapi: Error 401: not authorized to GetServiceAttachment, AccessDenied", }}, expectError: "failed to reconcile the Service Attachment: googleapi: Error 401: not authorized to GetServiceAttachment, AccessDenied", + }, { // Ready condition should be updated when serviceAttachmentModified + name: "serviceAttachmentModified", + cd: cdBuilder.Build( + testcd.WithGCPPlatform(&hivev1gcp.Platform{Region: testRegion}), + testcd.WithGCPPlatformStatus(&hivev1gcp.PlatformStatus{PrivateServiceConnect: &hivev1gcp.PrivateServiceConnectStatus{ + ServiceAttachmentSubnet: mockSubnet.SelfLink, + ServiceAttachmentFirewall: mockFirewall.SelfLink, + }}), + testcd.WithClusterMetadata(&hivev1.ClusterMetadata{InfraID: testInfraID}), + ), + config: &hivev1.GCPPrivateServiceConnectConfig{ + EndpointVPCInventory: []hivev1.GCPPrivateServiceConnectInventory{{ + Network: mockNetwork.Name, + Subnets: []hivev1.GCPPrivateServiceConnectSubnet{{ + Subnet: mockSubnet.SelfLink, + Region: testRegion, + }}, + }}, + }, + gcpClientConfig: func(m *mockclient.MockClient) { + m.EXPECT().GetSubnet(gomock.Any(), gomock.Any(), gomock.Any()).Return(mockSubnet, nil) + m.EXPECT().ListAddresses(gomock.Any(), gomock.Any()).Return(&compute.AddressList{ + Items: []*compute.Address{mockAddress}, + }, nil) + m.EXPECT().GetForwardingRule(gomock.Any(), gomock.Any()).Return(mockForwardingRule, nil) + m.EXPECT().GetNetwork(gomock.Any()).Return(mockNetwork, nil) + m.EXPECT().GetSubnet(gomock.Any(), gomock.Any(), gomock.Any()).Return(mockSubnet, nil) + m.EXPECT().GetFirewall(gomock.Any()).Return(mockFirewall, nil) + m.EXPECT().GetProjectName().Return(testProjectName) + m.EXPECT().GetServiceAttachment(gomock.Any(), gomock.Any()).Return(nil, newGoogleApiError(http.StatusNotFound, "NotFound", "ServiceAttachment not found")) + m.EXPECT().CreateServiceAttachment(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(mockServiceAttachment, nil) + }, + expect: reconcile.Result{Requeue: true}, + expectConditions: []hivev1.ClusterDeploymentCondition{{ + Status: corev1.ConditionFalse, + Type: hivev1.PrivateLinkReadyClusterDeploymentCondition, + Reason: "ReconciledServiceAttachment", + Message: "reconciled the Service Attachment", + }}, expectStatus: &hivev1gcp.PrivateServiceConnectStatus{ ServiceAttachmentSubnet: mockSubnet.SelfLink, ServiceAttachmentFirewall: mockFirewall.SelfLink, + ServiceAttachment: mockServiceAttachment.SelfLink, }, }, { // There should be an error on ensureEndpointAddress failure name: "ensureEndpointAddress failure", cd: cdBuilder.Build( - testcd.WithGCPPlatform(&hivev1gcp.Platform{Region: testRegion}), - testcd.WithGCPPlatformStatus(&hivev1gcp.PlatformStatus{PrivateServiceConnect: &hivev1gcp.PrivateServiceConnectStatus{}}), testcd.WithClusterMetadata(&hivev1.ClusterMetadata{InfraID: testInfraID}), + testcd.WithGCPPlatform(&hivev1gcp.Platform{Region: testRegion}), + testcd.WithGCPPlatformStatus(&hivev1gcp.PlatformStatus{PrivateServiceConnect: &hivev1gcp.PrivateServiceConnectStatus{ + ServiceAttachmentSubnet: mockSubnet.SelfLink, + ServiceAttachmentFirewall: mockFirewall.SelfLink, + ServiceAttachment: mockServiceAttachment.SelfLink, + }}), ), config: &hivev1.GCPPrivateServiceConnectConfig{ EndpointVPCInventory: []hivev1.GCPPrivateServiceConnectInventory{{ @@ -744,17 +864,63 @@ func Test_Reconcile(t *testing.T) { Message: "googleapi: Error 401: not authorized to GetAddress, AccessDenied", }}, expectError: "failed to reconcile the Endpoint Address: googleapi: Error 401: not authorized to GetAddress, AccessDenied", + }, { // Ready condition should be updated when addressModified + name: "addressModified", + cd: cdBuilder.Build( + testcd.WithGCPPlatform(&hivev1gcp.Platform{Region: testRegion}), + testcd.WithGCPPlatformStatus(&hivev1gcp.PlatformStatus{PrivateServiceConnect: &hivev1gcp.PrivateServiceConnectStatus{ + ServiceAttachmentSubnet: mockSubnet.SelfLink, + ServiceAttachmentFirewall: mockFirewall.SelfLink, + ServiceAttachment: mockServiceAttachment.SelfLink, + }}), + testcd.WithClusterMetadata(&hivev1.ClusterMetadata{InfraID: testInfraID}), + ), + config: &hivev1.GCPPrivateServiceConnectConfig{ + EndpointVPCInventory: []hivev1.GCPPrivateServiceConnectInventory{{ + Network: mockNetwork.Name, + Subnets: []hivev1.GCPPrivateServiceConnectSubnet{{ + Subnet: mockSubnet.SelfLink, + Region: testRegion, + }}, + }}, + }, + gcpClientConfig: func(m *mockclient.MockClient) { + m.EXPECT().GetSubnet(gomock.Any(), gomock.Any(), gomock.Any()).Return(mockSubnet, nil) + m.EXPECT().ListAddresses(gomock.Any(), gomock.Any()).Return(&compute.AddressList{ + Items: []*compute.Address{mockAddress}, + }, nil) + m.EXPECT().GetForwardingRule(gomock.Any(), gomock.Any()).Return(mockForwardingRule, nil) + m.EXPECT().GetNetwork(gomock.Any()).Return(mockNetwork, nil) + m.EXPECT().GetSubnet(gomock.Any(), gomock.Any(), gomock.Any()).Return(mockSubnet, nil) + m.EXPECT().GetFirewall(gomock.Any()).Return(mockFirewall, nil) + m.EXPECT().GetServiceAttachment(gomock.Any(), gomock.Any()).Return(mockServiceAttachment, nil) + m.EXPECT().GetAddress(gomock.Any(), gomock.Any()).Return(nil, newGoogleApiError(http.StatusNotFound, "NotFound", "Address not found")) + m.EXPECT().CreateAddress(gomock.Any(), gomock.Any(), gomock.Any()).Return(mockAddress, nil) + }, + expect: reconcile.Result{Requeue: true}, + expectConditions: []hivev1.ClusterDeploymentCondition{{ + Status: corev1.ConditionFalse, + Type: hivev1.PrivateLinkReadyClusterDeploymentCondition, + Reason: "ReconciledEndpointAddress", + Message: "reconciled the Endpoint Address", + }}, expectStatus: &hivev1gcp.PrivateServiceConnectStatus{ ServiceAttachmentSubnet: mockSubnet.SelfLink, ServiceAttachmentFirewall: mockFirewall.SelfLink, ServiceAttachment: mockServiceAttachment.SelfLink, + EndpointAddress: mockAddress.SelfLink, }, }, { // There should be an error on ensureEndpoint failure name: "ensureEndpoint failure", cd: cdBuilder.Build( - testcd.WithGCPPlatform(&hivev1gcp.Platform{Region: testRegion}), - testcd.WithGCPPlatformStatus(&hivev1gcp.PlatformStatus{PrivateServiceConnect: &hivev1gcp.PrivateServiceConnectStatus{}}), testcd.WithClusterMetadata(&hivev1.ClusterMetadata{InfraID: testInfraID}), + testcd.WithGCPPlatform(&hivev1gcp.Platform{Region: testRegion}), + testcd.WithGCPPlatformStatus(&hivev1gcp.PlatformStatus{PrivateServiceConnect: &hivev1gcp.PrivateServiceConnectStatus{ + ServiceAttachmentSubnet: mockSubnet.SelfLink, + ServiceAttachmentFirewall: mockFirewall.SelfLink, + ServiceAttachment: mockServiceAttachment.SelfLink, + EndpointAddress: mockAddress.SelfLink, + }}), ), config: &hivev1.GCPPrivateServiceConnectConfig{ EndpointVPCInventory: []hivev1.GCPPrivateServiceConnectInventory{{ @@ -785,17 +951,16 @@ func Test_Reconcile(t *testing.T) { Message: "googleapi: Error 401: not authorized to GetForwardingRule, AccessDenied", }}, expectError: "failed to reconcile the Endpoint: googleapi: Error 401: not authorized to GetForwardingRule, AccessDenied", - expectStatus: &hivev1gcp.PrivateServiceConnectStatus{ - ServiceAttachmentSubnet: mockSubnet.SelfLink, - ServiceAttachmentFirewall: mockFirewall.SelfLink, - ServiceAttachment: mockServiceAttachment.SelfLink, - EndpointAddress: mockAddress.SelfLink, - }, - }, { // An endpoint IP address should be returned on success - name: "success", + }, { // Ready condition should be updated when endpointModified + name: "endpointModified", cd: cdBuilder.Build( testcd.WithGCPPlatform(&hivev1gcp.Platform{Region: testRegion}), - testcd.WithGCPPlatformStatus(&hivev1gcp.PlatformStatus{PrivateServiceConnect: &hivev1gcp.PrivateServiceConnectStatus{}}), + testcd.WithGCPPlatformStatus(&hivev1gcp.PlatformStatus{PrivateServiceConnect: &hivev1gcp.PrivateServiceConnectStatus{ + ServiceAttachmentSubnet: mockSubnet.SelfLink, + ServiceAttachmentFirewall: mockFirewall.SelfLink, + ServiceAttachment: mockServiceAttachment.SelfLink, + EndpointAddress: mockAddress.SelfLink, + }}), testcd.WithClusterMetadata(&hivev1.ClusterMetadata{InfraID: testInfraID}), ), config: &hivev1.GCPPrivateServiceConnectConfig{ @@ -818,8 +983,10 @@ func Test_Reconcile(t *testing.T) { m.EXPECT().GetFirewall(gomock.Any()).Return(mockFirewall, nil) m.EXPECT().GetServiceAttachment(gomock.Any(), gomock.Any()).Return(mockServiceAttachment, nil) m.EXPECT().GetAddress(gomock.Any(), gomock.Any()).Return(mockAddress, nil) - m.EXPECT().GetForwardingRule(gomock.Any(), gomock.Any()).Return(mockForwardingRule, nil) + m.EXPECT().GetForwardingRule(gomock.Any(), gomock.Any()).Return(nil, newGoogleApiError(http.StatusNotFound, "NotFound", "ForwardingRule not found")) + m.EXPECT().CreateForwardingRule(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(mockForwardingRule, nil) }, + expect: reconcile.Result{Requeue: true}, expectConditions: []hivev1.ClusterDeploymentCondition{{ Status: corev1.ConditionFalse, Type: hivev1.PrivateLinkReadyClusterDeploymentCondition, @@ -833,6 +1000,41 @@ func Test_Reconcile(t *testing.T) { EndpointAddress: mockAddress.SelfLink, Endpoint: mockForwardingRule.SelfLink, }, + }, { // An endpoint IP address should be returned on success + name: "success", + cd: cdBuilder.Build( + testcd.WithClusterMetadata(&hivev1.ClusterMetadata{InfraID: testInfraID}), + testcd.WithGCPPlatform(&hivev1gcp.Platform{Region: testRegion}), + testcd.WithGCPPlatformStatus(&hivev1gcp.PlatformStatus{PrivateServiceConnect: &hivev1gcp.PrivateServiceConnectStatus{ + ServiceAttachmentSubnet: mockSubnet.SelfLink, + ServiceAttachmentFirewall: mockFirewall.SelfLink, + ServiceAttachment: mockServiceAttachment.SelfLink, + EndpointAddress: mockAddress.SelfLink, + Endpoint: mockForwardingRule.SelfLink, + }}), + ), + config: &hivev1.GCPPrivateServiceConnectConfig{ + EndpointVPCInventory: []hivev1.GCPPrivateServiceConnectInventory{{ + Network: mockNetwork.Name, + Subnets: []hivev1.GCPPrivateServiceConnectSubnet{{ + Subnet: mockSubnet.SelfLink, + Region: testRegion, + }}, + }}, + }, + gcpClientConfig: func(m *mockclient.MockClient) { + m.EXPECT().GetSubnet(gomock.Any(), gomock.Any(), gomock.Any()).Return(mockSubnet, nil) + m.EXPECT().ListAddresses(gomock.Any(), gomock.Any()).Return(&compute.AddressList{ + Items: []*compute.Address{mockAddress}, + }, nil) + m.EXPECT().GetForwardingRule(gomock.Any(), gomock.Any()).Return(mockForwardingRule, nil) + m.EXPECT().GetNetwork(gomock.Any()).Return(mockNetwork, nil) + m.EXPECT().GetSubnet(gomock.Any(), gomock.Any(), gomock.Any()).Return(mockSubnet, nil) + m.EXPECT().GetFirewall(gomock.Any()).Return(mockFirewall, nil) + m.EXPECT().GetServiceAttachment(gomock.Any(), gomock.Any()).Return(mockServiceAttachment, nil) + m.EXPECT().GetAddress(gomock.Any(), gomock.Any()).Return(mockAddress, nil) + m.EXPECT().GetForwardingRule(gomock.Any(), gomock.Any()).Return(mockForwardingRule, nil) + }, expectRecord: &actuator.DnsRecord{IpAddress: []string{mockAddress.Address}}, }} @@ -863,10 +1065,9 @@ func Test_Reconcile(t *testing.T) { } assert.Equal(t, test.expectRecord, dnsRecord) - if test.expectStatus == nil { - test.expectStatus = &hivev1gcp.PrivateServiceConnectStatus{} + if test.expectStatus != nil { + assert.Equal(t, test.expectStatus, test.cd.Status.Platform.GCP.PrivateServiceConnect) } - assert.Equal(t, test.expectStatus, test.cd.Status.Platform.GCP.PrivateServiceConnect) curr := &hivev1.ClusterDeployment{} fakeClient := *gcpLinkActuator.client