fix(provider): aws-sd provider null pointer

[ivankatliarchuk]

Description

Partially Fixes #5202

Relates #5167

^ In the issue is missing HOW to reproduce, so rely mainly on test fixtures

The issue fixes a case, when record exist, but not managed by current external-dns, aka missing label aws-sd-description. In that case we do a safe skip. Current behaviour - null pointer and crash.

For update,create,delete etc, required labels managed here

external-dns/registry/aws_sd_registry.go

Lines 84 to 100 in 4ea5f9d

	sdr.updateLabels(filteredChanges.Create)
	sdr.updateLabels(filteredChanges.UpdateNew)
	sdr.updateLabels(filteredChanges.UpdateOld)
	sdr.updateLabels(filteredChanges.Delete)
	return sdr.provider.ApplyChanges(ctx, filteredChanges)
	}
	func (sdr *AWSSDRegistry) updateLabels(endpoints []*endpoint.Endpoint) {
	for _, ep := range endpoints {
	if ep.Labels == nil {
	ep.Labels = make(map[string]string)
	}
	ep.Labels[endpoint.OwnerLabelKey] = sdr.ownerID
	ep.Labels[endpoint.AWSSDDescriptionLabel] = ep.Labels.SerializePlain(false)
	}
	}

managed to capture null pointers with test fixtures.

• Split tests and test fixtures. Refactoring of tests, I could add them in pre-requsit pull request
• Added early returns
• Added tests for dry-run
• Skip record if not managed by external-dns

if len(resp.Instances) == 0 {
  if err := p.DeleteService(ctx, srv); err != nil {
	  log.Errorf("Failed to delete service %q, error: %s", *srv.Name, err)
  }
  continue
}
// 3rd party created a record, external-dns could not manage it
if srv.Description == nil {
  log.Warnf("Skipping service %q as owner id not configured", *srv.Name)
  continue
}

Another potential case for nil pointer is here

external-dns/provider/awssd/aws_sd.go

Line 418 in 4ea5f9d

Description: aws.String(ep.Labels[endpoint.AWSSDDescriptionLabel]),

, as ep.Labels could be null at this stage. Not changed, will see if there going to be a bug raised.

Checklist

• Unit tests updated
• End user documentation updated

[ivankatliarchuk]

/retitle fix(provider): aws-sd provider null pointer

[ivankatliarchuk]

/label tide/merge-method-squash

[mloiseleur]

@redbaron Do you think you can do a first review on this PR ?

[mloiseleur]

@JyothsnaGuduri @restlesswind Can you confirm this PR fixes your issue ?

To test from a PR:

gh repo clone kubernetes-sigs/external-dns
cd external-dns
gh pr checkout 5404
kubectl config use-context <context>
go run main.go \
  --kubeconfig=<kubeconfig_path> \
  --xxx=yyy

[JyothsnaGuduri]

I can confirm this PR changes work fine,

I used following command after cloning the repo and PR branch

AWS_PROFILE=<aws-profile> go run main.go --kubeconfig=$HOME/.kube/config --namespace=external-dns --log-level=info --log-format=text --interval=10m --source=service --source=ingress --policy=upsert-only --registry=aws-sd --txt-owner-id=<owner-id> --domain-filter=<domain-filter> --provider=aws-sd

find the logs below

INFO[0000] config: {APIServerURL: KubeConfig:/Users/<user>/.kube/config RequestTimeout:30s DefaultTargets:[] ContourLoadBalancerService:heptio-contour/contour GlooNamespace:gloo-system SkipperRouteGroupVersion:zalando.org/v1 Sources:[service ingress] Namespace:external-dns AnnotationFilter: LabelFilter: FQDNTemplate: CombineFQDNAndAnnotation:false IgnoreHostnameAnnotation:false IgnoreIngressTLSSpec:false IgnoreIngressRulesSpec:false GatewayNamespace: GatewayLabelFilter: Compatibility: PublishInternal:false PublishHostIP:false AlwaysPublishNotReadyAddresses:false ConnectorSourceServer:localhost:8080 Provider:aws-sd GoogleProject: GoogleBatchChangeSize:1000 GoogleBatchChangeInterval:1s GoogleZoneVisibility: DomainFilter:[<domainFilter>] ExcludeDomains:[] RegexDomainFilter: RegexDomainExclusion: ZoneNameFilter:[] ZoneIDFilter:[] TargetNetFilter:[] ExcludeTargetNets:[] AlibabaCloudConfigFile:/etc/kubernetes/alibaba-cloud.json AlibabaCloudZoneType: AWSZoneType: AWSZoneTagFilter:[] AWSAssumeRole: AWSAssumeRoleExternalID: AWSBatchChangeSize:1000 AWSBatchChangeInterval:1s AWSEvaluateTargetHealth:true AWSAPIRetries:3 AWSPreferCNAME:false AWSZoneCacheDuration:0s AWSSDServiceCleanup:false AzureConfigFile:/etc/kubernetes/azure.json AzureResourceGroup: AzureSubscriptionID: AzureUserAssignedIdentityClientID: BluecatDNSConfiguration: BluecatConfigFile:/etc/kubernetes/bluecat.json BluecatDNSView: BluecatGatewayHost: BluecatRootZone: BluecatDNSServerName: BluecatDNSDeployType:no-deploy BluecatSkipTLSVerify:false CloudflareProxied:false CloudflareDNSRecordsPerPage:100 CoreDNSPrefix:/skydns/ RcodezeroTXTEncrypt:false AkamaiServiceConsumerDomain: AkamaiClientToken: AkamaiClientSecret: AkamaiAccessToken: AkamaiEdgercPath: AkamaiEdgercSection: InfobloxGridHost: InfobloxWapiPort:443 InfobloxWapiUsername:admin InfobloxWapiPassword: InfobloxWapiVersion:2.3.1 InfobloxSSLVerify:true InfobloxView: InfobloxMaxResults:0 InfobloxFQDNRegEx: InfobloxNameRegEx: InfobloxCreatePTR:false InfobloxCacheDuration:0 DynCustomerName: DynUsername: DynPassword: DynMinTTLSeconds:0 OCIConfigFile:/etc/kubernetes/oci.yaml InMemoryZones:[] OVHEndpoint:ovh-eu OVHApiRateLimit:20 PDNSServer:http://localhost:8081 PDNSAPIKey: PDNSTLSEnabled:false TLSCA: TLSClientCert: TLSClientCertKey: Policy:upsert-only Registry:aws-sd TXTOwnerID:eks-sandbox TXTPrefix: TXTSuffix: Interval:10m0s MinEventSyncInterval:5s Once:false DryRun:false UpdateEvents:false LogFormat:text MetricsAddress::7979 LogLevel:info TXTCacheInterval:0s TXTWildcardReplacement: ExoscaleEndpoint:https://api.exoscale.ch/dns ExoscaleAPIKey: ExoscaleAPISecret: CRDSourceAPIVersion:externaldns.k8s.io/v1alpha1 CRDSourceKind:DNSEndpoint ServiceTypeFilter:[] CFAPIEndpoint: CFUsername: CFPassword: RFC2136Host: RFC2136Port:0 RFC2136Zone: RFC2136Insecure:false RFC2136GSSTSIG:false RFC2136KerberosRealm: RFC2136KerberosUsername: RFC2136KerberosPassword: RFC2136TSIGKeyName: RFC2136TSIGSecret: RFC2136TSIGSecretAlg: RFC2136TAXFR:false RFC2136MinTTL:0s RFC2136BatchChangeSize:50 NS1Endpoint: NS1IgnoreSSL:false NS1MinTTLSeconds:0 TransIPAccountName: TransIPPrivateKeyFile: DigitalOceanAPIPageSize:50 ManagedDNSRecordTypes:[A CNAME] GoDaddyAPIKey: GoDaddySecretKey: GoDaddyTTL:0 GoDaddyOTE:false OCPRouterName: IBMCloudProxied:false IBMCloudConfigFile:/etc/kubernetes/ibmcloud.json TencentCloudConfigFile:/etc/kubernetes/tencent-cloud.json TencentCloudZoneType: PiholeServer: PiholePassword: PiholeTLSInsecureSkipVerify:false PluralCluster: PluralProvider:} 
INFO[0000] Instantiating new Kubernetes client          
INFO[0000] Using kubeConfig                             
INFO[0000] Created Kubernetes client https://<k8s-cluster>.gr7.us-west-2.eks.amazonaws.com 
INFO[0004] All records are already up to date  

Hope this helps

[mloiseleur]

Thanks 👍.
@ivankatliarchuk Once you have rebased, I think we can merge it.

[mloiseleur]

/approve
/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mloiseleur

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [mloiseleur]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment