Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: HIVE-2687: Privatelink improvements #2528

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

WIP: HIVE-2687: Privatelink improvements #2528

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
651f14c
privatelink: add privatelink controller unit tests
jstuever Nov 13, 2024
878ac0f
privatelink: ready and failed conditions logged only when changed
jstuever Dec 12, 2024
5fd4267
privatelink: ShouldSync returns false when privatelink is disabled
jstuever Dec 12, 2024
20cdfee
privatelink: actuator reconcile functions to requeue on every change
jstuever Dec 13, 2024
b90104b
privatelink: selectHostedZoneVPC to return a pointer to the selected vpc
jstuever Dec 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions pkg/controller/privatelink/actuator/actuator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
hivev1 "github.com/openshift/hive/apis/hive/v1"
)

//go:generate mockgen -source=./actuator.go -destination=./mock/actuator_generated.go -package=mock

type Actuator interface {

// Cleanup cleans up the cloud resources.
Expand Down
128 changes: 118 additions & 10 deletions pkg/controller/privatelink/actuator/awsactuator/awshubactuator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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")
Expand All @@ -177,13 +179,19 @@ 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
}

// ShouldSync is the actuator interface to determine if there are changes that need to be made.
func (a *AWSHubActuator) ShouldSync(cd *hivev1.ClusterDeployment) bool {
if cd.Spec.Platform.AWS == nil ||
cd.Spec.Platform.AWS.PrivateLink == nil ||
!cd.Spec.Platform.AWS.PrivateLink.Enabled {
return false
}
return cd.Status.Platform == nil ||
cd.Status.Platform.AWS == nil ||
cd.Status.Platform.AWS.PrivateLink == nil ||
Expand All @@ -209,7 +217,7 @@ func (a *AWSHubActuator) ensureHostedZone(cd *hivev1.ClusterDeployment, metadata
return false, "", err
}

newHzID, err := a.createHostedZone(&selectedVPC, apiDomain)
newHzID, err := a.createHostedZone(selectedVPC, apiDomain)
if err != nil {
return false, "", err
}
Expand Down Expand Up @@ -369,11 +377,113 @@ 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 {
continue
}
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{
Expand Down Expand Up @@ -599,9 +709,7 @@ func (a *AWSHubActuator) getEndpointVPC(cd *hivev1.ClusterDeployment, metadata *
return endpointVPC, nil
}

func (a *AWSHubActuator) selectHostedZoneVPC(cd *hivev1.ClusterDeployment, metadata *hivev1.ClusterMetadata, logger log.FieldLogger) (hivev1.AWSAssociatedVPC, error) {
selectedVPC := hivev1.AWSAssociatedVPC{}

func (a *AWSHubActuator) selectHostedZoneVPC(cd *hivev1.ClusterDeployment, metadata *hivev1.ClusterMetadata, logger log.FieldLogger) (*hivev1.AWSAssociatedVPC, error) {
// For clusterdeployments that are on AWS, use the VPCEndpoint VPC
if cd.Status.Platform != nil &&
cd.Status.Platform.AWS != nil &&
Expand All @@ -610,29 +718,29 @@ func (a *AWSHubActuator) selectHostedZoneVPC(cd *hivev1.ClusterDeployment, metad

endpointVPC, err := a.getEndpointVPC(cd, metadata)
if err != nil {
return selectedVPC, errors.Wrap(err, "error getting Endpoint VPC")
return nil, errors.Wrap(err, "error getting Endpoint VPC")
}

if endpointVPC.VPCID == "" {
return selectedVPC, errors.New("unable to select Endpoint VPC: Endpoint not found")
return nil, errors.New("unable to select Endpoint VPC: Endpoint not found")
}

return endpointVPC, nil
return &endpointVPC, nil
}

associatedVPCS, err := a.getAssociatedVPCs(cd, metadata, logger)
if err != nil {
return selectedVPC, errors.Wrap(err, "error getting associated VPCs")
return nil, errors.Wrap(err, "error getting associated VPCs")
}

// Select the first associatedVPC that uses the primary AWS PrivateLink credential.
// This is necessary because a Hosted Zone can only be created using a VPC owned by the same account.
for _, associatedVPC := range associatedVPCS {
if associatedVPC.CredentialsSecretRef == nil || *associatedVPC.CredentialsSecretRef == a.config.CredentialsSecretRef {
return associatedVPC, nil
return &associatedVPC, nil
}
}

// No VPCs found that match the criteria, return an error.
return selectedVPC, errors.New("unable to find an associatedVPC that uses the primary AWS PrivateLink credentials")
return nil, errors.New("unable to find an associatedVPC that uses the primary AWS PrivateLink credentials")
}
Loading