fix(endpoint): domains handling with idna

[ivankatliarchuk]

What does it do ?

• We need custom idna profile in multiple locations
• TXT records not deleted for idna affected records
• records with * not handled correctly
• for aws provider on DryRun added early return to reduce nesting

Follow-up most likely

• AWS provider HostedZoneId should be just an ID, without prefix. Example terraform https://github.com/hashicorp/terraform-provider-aws/blob/47afa5c147c6c5efede000e47d5d42483a0da928/internal/service/route53/record.go#L980 and API https://docs.aws.amazon.com/Route53/latest/APIReference/API_ChangeResourceRecordSets.html

Motivation

Fixes #5649
Fixes #5602

More

• Yes, this PR title follows Conventional Commits
• Yes, I added unit tests
• Yes, I updated end user documentation accordingly

Current

With the fix

records added

records deleted

same for records with * and _

[ivankatliarchuk]

Hi @a-pallari , @sathieu and @valerian-roche any chence to confirm the fix works?

If needed, see how to test a PR ?.

[valerian-roche]

We use roughly the same fix successfully in our fork.
The main difference is the Profile becoming a shared, reused instance. The structure methods have pointer receiver but from a quick read of the code do not seem to alter it, so it should not be an issue

[k8s-ci-robot]

@valerian-roche: changing LGTM is restricted to collaborators

In response to this:

We use roughly the same fix successfully in our fork.
The main difference is the Profile becoming a shared, reused instance. The structure methods have pointer receiver but from a quick read of the code do not seem to alter it, so it should not be an issue

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[sathieu]

Hi @a-pallari , @sathieu and @valerian-roche any chence to confirm the fix works?

@ivankatliarchuk I won't have the time to do e2e test, but I can confirm that this removes the error:

package main

import (
	"fmt"

	"golang.org/x/net/idna"
)

func main() {
	fmt.Println(idna.Lookup.ToUnicode("*.foo.example.org"))
	Profile := idna.New(
		idna.MapForLookup(),
		idna.Transitional(true),
		idna.StrictDomainName(false),
	)
	fmt.Println(Profile.ToUnicode("*.foo.example.org"))

}

*.foo.example.org idna: disallowed rune U+002A
*.foo.example.org <nil>

Program exited.

See on Go Playground

[ivankatliarchuk]

#5649 (comment)

Hi @RichardoC you could try this fix and validate?

how to test a PR ?.

[mloiseleur]

/lgtm

[RichardoC]

#5649 (comment)

Hi @RichardoC you could try this fix and validate?

how to test a PR ?.

Unfortunately not. The cluster where I see the issue isn't somewhere I can install unreleased software. Sorry!

[ivankatliarchuk]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ivankatliarchuk, valerian-roche

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 [ivankatliarchuk]

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