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.

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

feat(txt-registry): add option to use only new format #4946

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions docs/flags.md
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@
| `--txt-wildcard-replacement=""` | When using the TXT registry, a custom string that's used instead of an asterisk for TXT records corresponding to wildcard DNS records (optional) |
| `--[no-]txt-encrypt-enabled` | When using the TXT registry, set if TXT records should be encrypted before stored (default: disabled) |
| `--txt-encrypt-aes-key=""` | When using the TXT registry, set TXT record decryption and encryption 32 byte aes key (required when --txt-encrypt=true) |
| `--[no-]txt-new-format-only` | When using the TXT registry, only use new format records which include record type information (e.g., prefix: 'a-'). Reduces number of TXT records (default: disabled) |
| `--dynamodb-region=""` | When using the DynamoDB registry, the AWS region of the DynamoDB table (optional) |
| `--dynamodb-table="external-dns"` | When using the DynamoDB registry, the name of the DynamoDB table (default: "external-dns") |
| `--txt-cache-interval=0s` | The interval between cache synchronizations in duration format (default: disabled) |
Expand Down
29 changes: 29 additions & 0 deletions docs/registry/txt.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,35 @@
The TXT registry is the default registry.
It stores DNS record metadata in TXT records, using the same provider.

## Record Format Options
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you include a subsection on how to handle the migration from two records to a single record? Specifically, what are the steps involved in this consolidation, and will the external DNS automatically delete the old records or will manual cleanup be necessary?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now i haven't thought of any automatic cleanup, so for now it would be a manual cleanup for the old format TXT records.

Is that an okay approach, or would you like it to happen automatically?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mentioning this in the documentation likely is enough.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added an extra section describing a manual migration flow.

The TXT registry supports two formats for storing DNS record metadata:
- Legacy format: Creates a TXT record without record type information
- New format: Creates a TXT record with record type information (e.g., 'a-' prefix for A records)

By default, the TXT registry creates records in both formats for backwards compatibility. You can configure it to use only the new format by using the `--txt-new-format-only` flag. This reduces the number of TXT records created, which can be helpful when working with provider-specific record limits.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a bit more information about this behaviour as well? e.g. why external-dns creates two records

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have the history of why there are 2 formats, but would be glad to add some more text about it.

We just reached a point in my team at work where we would like to get rid of our duplicate TXT records in all of our DNS zones.


Note: The following record types always use only the new format regardless of this setting:
- AAAA records
- Encrypted TXT records (when using `--txt-encrypt-enabled`)

Example:
```sh
# Default behavior - creates both formats
external-dns --provider=aws --source=ingress --managed-record-types=A,TXT

# Only create new format records (alongside other required flags)
external-dns --provider=aws --source=ingress --managed-record-types=A,TXT --txt-new-format-only
```
The `--txt-new-format-only` flag should be used in addition to your existing external-dns configuration flags. It does not implicitly configure TXT record handling - you still need to specify `--managed-record-types=TXT` if you want external-dns to manage TXT records.

### Migration to New Format Only
When transitioning from dual-format to new-format-only records:
- Ensure all your `external-dns` instances support the new format
- Enable the `--txt-new-format-only` flag on your external-dns instances
Manually clean up any existing legacy format TXT records from your DNS provider

Note: `external-dns` will not automatically remove legacy format records when switching to new-format-only mode. You'll need to clean up the old records manually if desired.

## Prefixes and Suffixes

In order to avoid having the registry TXT records collide with
Expand Down
2 changes: 1 addition & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ func main() {
case "noop":
r, err = registry.NewNoopRegistry(p)
case "txt":
r, err = registry.NewTXTRegistry(p, cfg.TXTPrefix, cfg.TXTSuffix, cfg.TXTOwnerID, cfg.TXTCacheInterval, cfg.TXTWildcardReplacement, cfg.ManagedDNSRecordTypes, cfg.ExcludeDNSRecordTypes, cfg.TXTEncryptEnabled, []byte(cfg.TXTEncryptAESKey))
r, err = registry.NewTXTRegistry(p, cfg.TXTPrefix, cfg.TXTSuffix, cfg.TXTOwnerID, cfg.TXTCacheInterval, cfg.TXTWildcardReplacement, cfg.ManagedDNSRecordTypes, cfg.ExcludeDNSRecordTypes, cfg.TXTEncryptEnabled, []byte(cfg.TXTEncryptAESKey), cfg.TXTNewFormatOnly)
case "aws-sd":
r, err = registry.NewAWSSDRegistry(p, cfg.TXTOwnerID)
default:
Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/externaldns/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ type Config struct {
TXTSuffix string
TXTEncryptEnabled bool
TXTEncryptAESKey string `secure:"yes"`
TXTNewFormatOnly bool
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it be possible to update types_test.go as well?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added tests for the new flag now

Interval time.Duration
MinEventSyncInterval time.Duration
Once bool
Expand Down Expand Up @@ -299,6 +300,7 @@ var defaultConfig = &Config{
MinEventSyncInterval: 5 * time.Second,
TXTEncryptEnabled: false,
TXTEncryptAESKey: "",
TXTNewFormatOnly: false,
Interval: time.Minute,
Once: false,
DryRun: false,
Expand Down Expand Up @@ -591,6 +593,7 @@ func App(cfg *Config) *kingpin.Application {
app.Flag("txt-wildcard-replacement", "When using the TXT registry, a custom string that's used instead of an asterisk for TXT records corresponding to wildcard DNS records (optional)").Default(defaultConfig.TXTWildcardReplacement).StringVar(&cfg.TXTWildcardReplacement)
app.Flag("txt-encrypt-enabled", "When using the TXT registry, set if TXT records should be encrypted before stored (default: disabled)").BoolVar(&cfg.TXTEncryptEnabled)
app.Flag("txt-encrypt-aes-key", "When using the TXT registry, set TXT record decryption and encryption 32 byte aes key (required when --txt-encrypt=true)").Default(defaultConfig.TXTEncryptAESKey).StringVar(&cfg.TXTEncryptAESKey)
app.Flag("txt-new-format-only", "When using the TXT registry, only use new format records which include record type information (e.g., prefix: 'a-'). Reduces number of TXT records (default: disabled)").BoolVar(&cfg.TXTNewFormatOnly)
app.Flag("dynamodb-region", "When using the DynamoDB registry, the AWS region of the DynamoDB table (optional)").Default(cfg.AWSDynamoDBRegion).StringVar(&cfg.AWSDynamoDBRegion)
app.Flag("dynamodb-table", "When using the DynamoDB registry, the name of the DynamoDB table (default: \"external-dns\")").Default(defaultConfig.AWSDynamoDBTable).StringVar(&cfg.AWSDynamoDBTable)

Expand Down
10 changes: 7 additions & 3 deletions pkg/apis/externaldns/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ var (
AzureSubscriptionID: "",
CloudflareProxied: false,
CloudflareDNSRecordsPerPage: 100,
CloudflareRegionKey: "",
CloudflareRegionKey: "",
CoreDNSPrefix: "/skydns/",
AkamaiServiceConsumerDomain: "",
AkamaiClientToken: "",
Expand All @@ -97,6 +97,7 @@ var (
TXTOwnerID: "default",
TXTPrefix: "",
TXTCacheInterval: 0,
TXTNewFormatOnly: false,
Interval: time.Minute,
MinEventSyncInterval: 5 * time.Second,
Once: false,
Expand Down Expand Up @@ -176,7 +177,7 @@ var (
AzureSubscriptionID: "arg",
CloudflareProxied: true,
CloudflareDNSRecordsPerPage: 5000,
CloudflareRegionKey: "us",
CloudflareRegionKey: "us",
CoreDNSPrefix: "/coredns/",
AkamaiServiceConsumerDomain: "oooo-xxxxxxxxxxxxxxxx-xxxxxxxxxxxxxxxx.luna.akamaiapis.net",
AkamaiClientToken: "o184671d5307a388180fbf7f11dbdf46",
Expand All @@ -202,6 +203,7 @@ var (
TXTOwnerID: "owner-1",
TXTPrefix: "associated-txt-record",
TXTCacheInterval: 12 * time.Hour,
TXTNewFormatOnly: true,
Interval: 10 * time.Minute,
MinEventSyncInterval: 50 * time.Second,
Once: true,
Expand Down Expand Up @@ -338,6 +340,7 @@ func TestParseFlags(t *testing.T) {
"--txt-owner-id=owner-1",
"--txt-prefix=associated-txt-record",
"--txt-cache-interval=12h",
"--txt-new-format-only",
"--dynamodb-table=custom-table",
"--interval=10m",
"--min-event-sync-interval=50s",
Expand Down Expand Up @@ -399,7 +402,7 @@ func TestParseFlags(t *testing.T) {
"EXTERNAL_DNS_AZURE_SUBSCRIPTION_ID": "arg",
"EXTERNAL_DNS_CLOUDFLARE_PROXIED": "1",
"EXTERNAL_DNS_CLOUDFLARE_DNS_RECORDS_PER_PAGE": "5000",
"EXTERNAL_DNS_CLOUDFLARE_REGION_KEY": "us",
"EXTERNAL_DNS_CLOUDFLARE_REGION_KEY": "us",
"EXTERNAL_DNS_COREDNS_PREFIX": "/coredns/",
"EXTERNAL_DNS_AKAMAI_SERVICECONSUMERDOMAIN": "oooo-xxxxxxxxxxxxxxxx-xxxxxxxxxxxxxxxx.luna.akamaiapis.net",
"EXTERNAL_DNS_AKAMAI_CLIENT_TOKEN": "o184671d5307a388180fbf7f11dbdf46",
Expand Down Expand Up @@ -451,6 +454,7 @@ func TestParseFlags(t *testing.T) {
"EXTERNAL_DNS_TXT_OWNER_ID": "owner-1",
"EXTERNAL_DNS_TXT_PREFIX": "associated-txt-record",
"EXTERNAL_DNS_TXT_CACHE_INTERVAL": "12h",
"EXTERNAL_DNS_TXT_NEW_FORMAT_ONLY": "1",
"EXTERNAL_DNS_INTERVAL": "10m",
"EXTERNAL_DNS_MIN_EVENT_SYNC_INTERVAL": "50s",
"EXTERNAL_DNS_ONCE": "1",
Expand Down
24 changes: 18 additions & 6 deletions registry/txt.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,18 @@ type TXTRegistry struct {
// encrypt text records
txtEncryptEnabled bool
txtEncryptAESKey []byte

newFormatOnly bool
}

// NewTXTRegistry returns new TXTRegistry object
func NewTXTRegistry(provider provider.Provider, txtPrefix, txtSuffix, ownerID string, cacheInterval time.Duration, txtWildcardReplacement string, managedRecordTypes, excludeRecordTypes []string, txtEncryptEnabled bool, txtEncryptAESKey []byte) (*TXTRegistry, error) {
// NewTXTRegistry returns a new TXTRegistry object. When newFormatOnly is true, it will only
// generate new format TXT records, otherwise it generates both old and new formats for
// backwards compatibility.
func NewTXTRegistry(provider provider.Provider, txtPrefix, txtSuffix, ownerID string,
cacheInterval time.Duration, txtWildcardReplacement string,
managedRecordTypes, excludeRecordTypes []string,
txtEncryptEnabled bool, txtEncryptAESKey []byte,
newFormatOnly bool) (*TXTRegistry, error) {
if ownerID == "" {
return nil, errors.New("owner id cannot be empty")
}
Expand Down Expand Up @@ -88,6 +96,7 @@ func NewTXTRegistry(provider provider.Provider, txtPrefix, txtSuffix, ownerID st
excludeRecordTypes: excludeRecordTypes,
txtEncryptEnabled: txtEncryptEnabled,
txtEncryptAESKey: txtEncryptAESKey,
newFormatOnly: newFormatOnly,
}, nil
}

Expand Down Expand Up @@ -209,12 +218,14 @@ func (im *TXTRegistry) Records(ctx context.Context) ([]*endpoint.Endpoint, error
return endpoints, nil
}

// generateTXTRecord generates both "old" and "new" TXT records.
// Once we decide to drop old format we need to drop toTXTName() and rename toNewTXTName
// generateTXTRecord generates TXT records in either both formats (old and new) or new format only,
// depending on the newFormatOnly configuration. The old format is maintained for backwards
// compatibility but can be disabled to reduce the number of DNS records.
func (im *TXTRegistry) generateTXTRecord(r *endpoint.Endpoint) []*endpoint.Endpoint {
endpoints := make([]*endpoint.Endpoint, 0)

if !im.txtEncryptEnabled && !im.mapper.recordTypeInAffix() && r.RecordType != endpoint.RecordTypeAAAA {
// Create legacy format record by default unless newFormatOnly is true
if !im.newFormatOnly && !im.txtEncryptEnabled && !im.mapper.recordTypeInAffix() && r.RecordType != endpoint.RecordTypeAAAA {
// old TXT record format
txt := endpoint.NewEndpoint(im.mapper.toTXTName(r.DNSName), endpoint.RecordTypeTXT, r.Labels.Serialize(true, im.txtEncryptEnabled, im.txtEncryptAESKey))
if txt != nil {
Expand All @@ -224,7 +235,8 @@ func (im *TXTRegistry) generateTXTRecord(r *endpoint.Endpoint) []*endpoint.Endpo
endpoints = append(endpoints, txt)
}
}
// new TXT record format (containing record type)

// Always create new format record
recordType := r.RecordType
// AWS Alias records are encoded as type "cname"
if isAlias, found := r.GetProviderSpecificProperty("alias"); found && isAlias == "true" && recordType == endpoint.RecordTypeA {
Expand Down
Loading
Loading