Fix planning for multi-cluster dual stack record types #3747

cronik · 2023-06-28T17:48:04Z

When AAAA multi-target / dual stack support was
added via #2461 it broke ownership of domains across different clusters with different ingress records types.

For example if 2 clusters manage the same zone,
1 cluster uses A records and the other uses CNAME
records, when each record type is treated as a separate planning record, it will cause ownership to bounce back and forth and records to be constantly created and deleted.

Description

This change updates the planner to keep track of multiple
current records for a domain. This allows for A and AAAA
records to exist for a domain while allowing record type
changes.

The planner will ignore desired records for a domain that
represent conflicting record types allowed by RFC 1034 3.6.2.
For example if the desired records for a domain contains
a CNAME record plus any other record type no changes for
that domain will be planned.

The planner now contains an owned record filter provided
by the registry. This allows the planner to accurately plan
create updates when there are record type changes between
the current and desired endpoints. Without this filter the
planner could add create changes for domains not owned
by the controller.

Fixes #2461

Checklist

Unit tests updated
End user documentation updated

johngmyers

I don't think this is the right approach. It is not AAAA that is special, it is CNAME that is special per RFC 1034 3.6.2.

This approach is likely to stop working once #3605 lands.

To address this issue, the planner will need to resolve conflicts between CNAME and other types.

cronik · 2023-06-29T13:25:46Z

@johngmyers thanks for the review and the additional RFC context.

If I read between the lines in your comment, it sounds like you agree there is an issue with the current planner implementation, but my solution needs to be refactored around CNAME rather than a configurable set "dual-stack-record-types".

johngmyers · 2023-06-29T14:06:41Z

That is correct. My apologies, I intended that to be the text, not the subtext.

johngmyers

This doesn't seem correct. The conflict resolver doesn't appear to be examining the record types of the current and candidates. A CNAME should conflict with records of any type, but no two records of different types, neither type CNAME, should conflict with each other.

With one key exception: a CNAME that is an alias should not conflict with any records of different type.

johngmyers · 2023-06-30T05:29:46Z

plan/plan.go

+//		dog.com | [->1.1.1.2]          |  				             |  = delete (dog.com [-> 1.1.1.2])
+//		--------------------------------------------------------------
+//		cat.com | [->::1, ->1.1.1.3]   | [->1.1.1.3]  				 |  = update old (cat.com [-> ::1, -> 1.1.1.3]) new (cat.com [-> 1.1.1.3])
+//		--------------------------------------------------------------
+//		big.com | [->1.1.1.4]          | [->ing.elb.com]  		     |  = update old (big.com [-> 1.1.1.4]) new (big.com [-> ing.elb.com])


The spacing is off.

johngmyers · 2023-06-30T05:35:20Z

plan/plan.go

+		if len(row.current) > 0 && len(row.candidates) == 0 {
+			for _, current := range row.current {
+				changes.Delete = append(changes.Delete, current)
+			}
 		}

 		// TODO: allows record type change, which might not be supported by all dns providers


I think we can remove this comment.

johngmyers · 2023-06-30T06:07:28Z

Perhaps CNAME conflicts should be resolved as a post-processing step? For each row, if the type is not CNAME and there is a corresponding CNAME row with >1 candidate, save the row keyed by domain and identifier. Then for each saved <domain, identifier> tuple, determine whether the CNAME wins or one of the other records wins. Depending on who wins, delete either the candidates for the CNAME or the candidates for all the other types.

cronik · 2023-07-14T01:39:00Z

@johngmyers I took another cut at this. In order for the planner to correctly handle record type changes I had to introduce a owned record filter to the planner, sourced from the registry. This filter allows the planner to add create records only when the controller has a clear ownership claim on it. With this filter available the planning became much easier and the change sets more accurate.

With respect to the conflicting record types, I wasn't sure how to best resolve conflicts when the sources produce conflicting desired record types. I ended up punting on this by skipping any updates and logging a warning.

johngmyers · 2023-07-31T15:24:07Z

Moving filterOwnedRecords to the planner is a reasonable refactor, but it should just be moved, without an interface and cooperation from all the registries. This refactor should be a separate commit.

I suspect all the conflict resolution code should be pushed down into the ConflictResolver.

hasCandidateRecordTypeConflict() is not sufficient. There can be conflicts between a current that is not owned and candidates, without that current having a corresponding candidate of the same recordType.

Conflicts between CNAME and other record types should be resolved in favor of what exists in current. If nothing exists in current, then I propose CNAME should take precedence.

cronik · 2023-07-31T23:51:05Z

Moving filterOwnedRecords to the planner is a reasonable refactor, but it should just be moved, without an interface and cooperation from all the registries. This refactor should be a separate commit.

@johngmyers I don't follow the request to only move filterOwnedRecords to planner without cooperation from registries. The main reason for the cooperation is that not all registries filter by owner, namely the noop registry. The external dns design puts the responsibility of record ownership on the registries so it only seems natural that it would be a collaborator with the planner for ownership identification, just like it is for PropertyComparator and DomainFilter.

Regarding the interface, I extracted EndpointFilterInterface since that appeared to be consistent with existing patterns (i.e. DomainFilterInterface used by the plan DomainFilter) and reduced the code duplication spread across the registries. Can you expand on why the interface is not desired and what you would like to see in its place?

hasCandidateRecordTypeConflict() is not sufficient. There can be conflicts between a current that is not owned and candidates, without that current having a corresponding candidate of the same recordType.

I think these are covered in the unit test cases I added (See TestConflicting* and Test*ConflictingDesired tests in plan_test.go). The implementation is not looking for conflicts in current for a couple reasons. First, if there are conflicts in current that would likely indicate an issue with the current provider and not something we necessarily need to attempt to resolve. Second, if ownership is confirmed (which it is via OwnedRecordFilter) and there are no conflicts with the proposed candidate record types, then the conflict will be resolved naturally by applying the planed changes. If you have additional test cases in mind that I missed please indicate and I'll add them to the suite.

Conflicts between CNAME and other record types should be resolved in favor of what exists in current. If nothing exists in current, then I propose CNAME should take precedence.

I will add a new ConflictResolver method to resolve record type conflicts and the PerResource implementation will adopt this rule.

johngmyers · 2023-08-01T00:50:22Z

PropertyComparator is being removed in #3702. With the addition of the webhook provider, we don't want to be adding large numbers (per sync) of calls through the Provider/Registry interface.

The endpoint.OwnerLabelKey label is already the defined interface for registries to indicate ownership. We shouldn't be adding more, complicated mechanisms to do the same thing.

With the noop registry, the controller owns all records returned by the provider. We might need to modify the noop registry to add ownership labels to any returned records lacking them. Or we carve out a mechanism to tell the planner it shouldn't care about lack of ownership.

DomainFilterInterface isn't actually needed anymore; I will need to refactor it out.

The missing test case is if there is an existing unowned SRV and a candidate A, then the planner should create the A. There is no conflict in that case.

cronik · 2023-08-01T03:00:38Z

The filter interface pattern is getting refactored out, I get that now. But I'm still not clear exactly what you have in mind as a replacement. Since I'm not aware of all these refactorings or designs in the works, can you please be more specific in how you would like both the planner and the registries to deal with determining ownership? Should the planner be configured with the ownerID? If so, where should that come from? The controller doesn't have that information, it's currently only stored as a private field in the registry. If you layout the flow of information I can try to execute that rather than through trial and error.

The missing test case is if there is an existing unowned SRV and a candidate A, then the planner should create the A. There is no conflict in that case.

SRV is not one of the supported managed record types, so is that a valid use case for the planner? the managed records filter would remove those record types from analysis.

external-dns/pkg/apis/externaldns/types.go

Line 437 in bc61d4d

    
           app.Flag("managed-record-types", "Record types to manage; specify multiple times to include many; (default: A, AAAA, CNAME) (supported records: CNAME, A, AAAA, NS").Default("A", "AAAA", "CNAME").StringsVar(&cfg.ManagedDNSRecordTypes)

johngmyers · 2023-08-11T05:39:10Z

SRV is a supported managed record type; it is produced by the service source for NodePort services. The description of that flag is inaccurate.

DomainFilterInterface is being replaced with DomainFilter.

cfg.TXTOwnerID is common to all registries (despite the "TXT" prefix) and should be passed to the planner.

When AAAA multi-target / dual stack support was added via kubernetes-sigs#2461 it broke ownership of domains across different clusters with different ingress records types. For example if 2 clusters manage the same zone, 1 cluster uses A records and the other uses CNAME records, when each record type is treated as a separate planning record, it will cause ownership to bounce back and forth and records to be constantly created and deleted. This change updates the planner to keep track of multiple current records for a domain. This allows for A and AAAA records to exist for a domain while allowing record type changes. The planner will ignore desired records for a domain that represent conflicting record types allowed by RFC 1034 3.6.2. For example if the desired records for a domain contains a CNAME record plus any other record type no changes for that domain will be planned. The planner now contains an owned record filter provided by the registry. This allows the planner to accurately plan create updates when there are record type changes between the current and desired endpoints. Without this filter the planner could add create changes for domains not owned by the controller.

johngmyers · 2023-09-01T05:26:32Z

There's one special case where a CNAME record doesn't conflict with other record types: when there is a ProviderSpecificProperty providerSpecificAlias with value "true". That actually maps to an A record in the route53 provider, but due to existing ownership records it has to be represented as a CNAME record to the TXT registry (and thus planner, unless we want to special-case that in the txt registry).

cronik · 2023-09-02T20:09:51Z

There's one special case where a CNAME record doesn't conflict with other record types: when there is a ProviderSpecificProperty providerSpecificAlias with value "true". That actually maps to an A record in the route53 provider, but due to existing ownership records it has to be represented as a CNAME record to the TXT registry (and thus planner, unless we want to special-case that in the txt registry).

I don't really understand this special case. Can I just ignore CNAME endpoints with "alias" specific property "true" when search for conflicts?

johngmyers · 2023-09-02T22:38:39Z

@Raffo @szuecs @mloiseleur I'd like opinions as to how we should handle this problem:

This PR is trying to update the planner to deal with the fact that several providers enforce the RFC 1034 3.6.2 restriction against there being both CNAME and other record types for the same domain. So it's resolving the conflict between CNAME and other record types for the same domain. (Prior to #2461 there couldn't be multiple record types for the same domain.)

The problem is with AWS alias records. An AWS alias record of type A is represented to the planner as a record of type CNAME with an alias ProviderSpecificProperty of true. With this PR, that would conflict with an AWS alias record of type AAAA, whereas we would want both to be created.

I see two ways of fixing this:

Have the planner special-case CNAME records that have an alias ProviderSpecificProperty of true as not conflicting with records of other types (but possibly conflicting with CNAME records without such a ProviderSpecificProperty).
Change the Route53 provider to represent alias records of type A as endpoints of type A with an alias ProviderSpecificProperty of true.

The problem with (2) is that existing alias records of type A will have new-format TXT registry ownership records registered with the type "cname". So we would also have to add special-case code to the TXT registry to do this mapping.

I think it's worth doing (2) as that will eventually lead to simpler code, especially if we migrate off of the current "new format" TXT records.

johngmyers · 2023-09-03T01:01:54Z

Filed #3910 to address the problem of conflicts for AWS Aliases. This isn't actually a problem until #3605 lands, as AWS aliases are currently only represented to the planner as CNAME records. So this issue doesn't block this PR.

johngmyers

All minor stuff; we're getting close.

johngmyers · 2023-09-03T01:08:38Z

plan/conflict.go

+
+	// conflict was found, remove candiates of non-preferred record types
+	if cname && other {
+		log.Warnf("Domain %s contains conflicting record type candidates, keeping CNAME record", key.dnsName)


Warn is for deprecations; please use Infof

The "keeping CNAME record" part of the log message is no longer correct.

johngmyers · 2023-09-03T01:11:46Z

plan/conflict.go

+		records := map[string]*domainEndpoints{}
+		for recordType, recs := range row.records {
+			if recordType != endpoint.RecordTypeCNAME {
+				// policy is to prefer the A and AAAA record type when a conflict is found


Suggested change

// policy is to prefer the A and AAAA record type when a conflict is found

// policy is to prefer the non-CNAME record types when a conflict is found

johngmyers · 2023-09-03T01:12:44Z

plan/conflict.go

+		log.Warnf("Domain %s contains conflicting record type candidates, keeping CNAME record", key.dnsName)
+		records := map[string]*domainEndpoints{}
+		for recordType, recs := range row.records {
+			if recordType != endpoint.RecordTypeCNAME {


It would be clearer to invert the condition of the if statement, putting the CNAME case first.

johngmyers · 2023-09-03T02:29:45Z

plan/plan.go

 type planTableRow struct {
-	current    *endpoint.Endpoint
+	// current corresponds to the records currently occupying dns name on the dns provider. More than 1 record may


Suggested change

// current corresponds to the records currently occupying dns name on the dns provider. More than 1 record may

// current corresponds to the records currently occupying dns name on the dns provider. More than one record may

johngmyers · 2023-09-03T02:30:27Z

plan/plan.go

 type planTableRow struct {
-	current    *endpoint.Endpoint
+	// current corresponds to the records currently occupying dns name on the dns provider. More than 1 record may
+	// be represented here, for example A and AAAA. If current domain record is CNAME, no other record types are allowed


Suggested change

// be represented here, for example A and AAAA. If current domain record is CNAME, no other record types are allowed

// be represented here: for example A and AAAA. If the current domain record is a CNAME, no other record types are allowed

johngmyers · 2023-09-03T02:41:00Z

registry/txt.go

+		UpdateNew: endpoint.FilterEndpointsByOwnerID(im.ownerID, changes.UpdateNew),
+		UpdateOld: endpoint.FilterEndpointsByOwnerID(im.ownerID, changes.UpdateOld),
+		Delete:    endpoint.FilterEndpointsByOwnerID(im.ownerID, changes.Delete),


Is the filtering in the registry still needed now that the planner is filtering by OwnerID?

One might argue that the registry shouldn't assume the state of the arguments provided by the caller since technically plan.Changes is mutable. But practically speaking the controller is the only caller (except for tests) so it's probably a safe assumption. Would suggest making this optimization in a separate PR since it will likely require a significant changeset to cleanup the registry tests.

johngmyers · 2023-09-03T17:11:21Z

plan/conflict.go

+
+	// conflict was found, remove candiates of non-preferred record types
+	if cname && other {
+		log.Infof("Domain %s contains conflicting record type candidates, discarding CNAME record", key.dnsName)


Suggested change

log.Infof("Domain %s contains conflicting record type candidates, discarding CNAME record", key.dnsName)

log.Infof("Domain %s contains conflicting record type candidates; discarding CNAME record", key.dnsName)

johngmyers · 2023-09-03T17:11:42Z

/lgtm

endpoint.Targets.Less gives priority to A and AAAA over CNAME records so the plan record type conflict resolver should also prefer A/AAAA.

johngmyers · 2023-09-03T20:29:16Z

/lgtm

johngmyers · 2023-09-03T20:30:24Z

/assign @Raffo

johngmyers · 2023-09-15T05:09:14Z

/approve

k8s-ci-robot · 2023-09-15T05:09:21Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: johngmyers

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~~ [johngmyers]

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

max-blue · 2023-11-01T19:10:31Z

still seeing an error with chart version 6.28.0 and app version 0.13.6. Getting the error below:

time="2023-11-01T13:29:42Z" level=fatal msg="googleapi: Error 400: The resource record set 'entity.change.additions[www.beta.domain.com.][A]' is invalid because the DNS name 'www.beta.domain.com.' has a resource record set of the type 'CNAME'. A DNS name may have either one CNAME resource record set or resource record sets of other types, but not both., cnameResourceRecordSetConflict"

www. is CNAMED to a cloudfront distro outside of external-dns

leonardocaylent · 2024-02-09T15:44:38Z

@cronik This PR introduced #4241 , can you take a look? I described the issue using a single EKS cluster

cronik · 2024-02-17T15:48:31Z

@cronik This PR introduced #4241 , can you take a look? I described the issue using a single EKS cluster

@leonardocaylent Took a quick look at where the error is occurring and the logs from your issue and it seems to me like an issue with aws provider not batching planned changes correctly.

The log message provided seems to cut off the full "InvalidChangeBatch" reason and I'm not familiar with aws route 53, certainly not the details of change batch rules. But I can see from the logs that it appears to batch both hosted zone 1 and 2 changes together while before there was a "successful update" message between the desired change messages.

v0.14.0 contained changes to the AWS provider as well via #3910, maybe it's related to that given the working log messages attempt to adjust "cname-testapplication.internal.sandbox.yourdomain.com"?

leonardocaylent · 2024-02-18T18:17:30Z

@cronik This PR introduced #4241 , can you take a look? I described the issue using a single EKS cluster

@leonardocaylent Took a quick look at where the error is occurring and the logs from your issue and it seems to me like an issue with aws provider not batching planned changes correctly.

The log message provided seems to cut off the full "InvalidChangeBatch" reason and I'm not familiar with aws route 53, certainly not the details of change batch rules. But I can see from the logs that it appears to batch both hosted zone 1 and 2 changes together while before there was a "successful update" message between the desired change messages.

v0.14.0 contained changes to the AWS provider as well via #3910, maybe it's related to that given the working log messages attempt to adjust "cname-testapplication.internal.sandbox.yourdomain.com"?

@cronik I compiled and created Docker Images from master on commits 030342f and 65db0c7 and I confirmed the one that introduced the change is this PR #3747 .
I also tested a docker image from #3910 but it's also failing because this PR (3747) was merged first.
About debugging this issue: Yes, it seems like something is being malformed before sending the request to AWS but I'm not sure if setting external-dns logs to debug will give more information or some special debugging lines needs to be added in order to achieve that. I think maybe some of the changes to aws_sd_registry.go or txt.go introduced this behaviour.

This is AWS Cloudtrail for ChangeResourceRecordSets:

"errorCode": "InvalidChangeBatch",
    "errorMessage": "[The request contains an invalid set of changes for a resource record set 'A testapplication.internal.dev.yourdomain.com.', The request contains an invalid set of changes for a resource record set 'TXT testapplication.internal.dev.yourdomain.com.', The request contains an invalid set of changes for a resource record set 'TXT cname-testapplication.internal.dev.yourdomain.com.']"

I don't know if this use case could be added to the test suite (since the behavior needs to be tested with the AWS API and cannot be faked) because it's really common and we should get coverage for that (if possible)

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 28, 2023

k8s-ci-robot requested review from seanmalloy and szuecs June 28, 2023 17:48

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jun 28, 2023

johngmyers suggested changes Jun 29, 2023

View reviewed changes

k8s-ci-robot assigned johngmyers Jun 29, 2023

cronik force-pushed the bugfix/dual-stack-records branch from ea36062 to 572c341 Compare June 30, 2023 00:49

johngmyers suggested changes Jun 30, 2023

View reviewed changes

johngmyers mentioned this pull request Jul 5, 2023

external-dns not ignoring pre-existing DNS records it does not have ownership of #3715

Closed

cronik force-pushed the bugfix/dual-stack-records branch from 572c341 to c5047c0 Compare July 14, 2023 01:30

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 14, 2023

cronik force-pushed the bugfix/dual-stack-records branch from c5047c0 to 1e251b7 Compare July 14, 2023 02:01

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 20, 2023

cronik force-pushed the bugfix/dual-stack-records branch from 47b5fda to 03379e4 Compare July 23, 2023 03:05

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 23, 2023

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 11, 2023

cronik added 3 commits August 15, 2023 19:37

Remove unused validateOwner plan test function

635da46

cronik force-pushed the bugfix/dual-stack-records branch from 8a56a39 to 635da46 Compare August 17, 2023 03:47

johngmyers mentioned this pull request Sep 3, 2023

Represent AWS Alias records as record type A #3910

Merged

2 tasks

johngmyers suggested changes Sep 3, 2023

View reviewed changes

cronik force-pushed the bugfix/dual-stack-records branch from 9cc5da4 to 4c8b5dc Compare September 3, 2023 14:25

johngmyers reviewed Sep 3, 2023

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 3, 2023

Plan record type conflict should prefer A and AAAA records

55ae13e

endpoint.Targets.Less gives priority to A and AAAA over CNAME records so the plan record type conflict resolver should also prefer A/AAAA.

cronik force-pushed the bugfix/dual-stack-records branch from 4c8b5dc to 55ae13e Compare September 3, 2023 19:58

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 3, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 3, 2023

k8s-ci-robot assigned Raffo Sep 3, 2023

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 15, 2023

k8s-ci-robot merged commit 030342f into kubernetes-sigs:master Sep 15, 2023

wbh1 mentioned this pull request Oct 9, 2023

Linode DNS broken since v0.11 #3924

Closed

bo0tzz mentioned this pull request Jan 16, 2024

External DNS refuses to create CNAME record #4185

Closed

leonardocaylent mentioned this pull request Feb 9, 2024

v.0.14.0 [AWS] External-DNS cannot remove records from 2 Route 53 hosted zones (InvalidChangeBatch: [The request contains an invalid set of changes]) #4241

Closed

leonardocaylent mentioned this pull request Mar 16, 2024

fix: duplicated endpoint per hosted zone #4296

Merged

2 tasks

leonardocaylent mentioned this pull request Mar 26, 2024

[v0.14.0] Unit tests on endpoint_test.go need a renaming #4339

Closed

	// policy is to prefer the A and AAAA record type when a conflict is found
	// policy is to prefer the non-CNAME record types when a conflict is found

	// current corresponds to the records currently occupying dns name on the dns provider. More than 1 record may
	// current corresponds to the records currently occupying dns name on the dns provider. More than one record may

	// be represented here, for example A and AAAA. If current domain record is CNAME, no other record types are allowed
	// be represented here: for example A and AAAA. If the current domain record is a CNAME, no other record types are allowed

	log.Infof("Domain %s contains conflicting record type candidates, discarding CNAME record", key.dnsName)
	log.Infof("Domain %s contains conflicting record type candidates; discarding CNAME record", key.dnsName)

Fix planning for multi-cluster dual stack record types #3747

Fix planning for multi-cluster dual stack record types #3747

Conversation

cronik commented Jun 28, 2023 • edited Loading

johngmyers left a comment

Choose a reason for hiding this comment

cronik commented Jun 29, 2023

johngmyers commented Jun 29, 2023

johngmyers left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johngmyers commented Jun 30, 2023

cronik commented Jul 14, 2023

johngmyers commented Jul 31, 2023

cronik commented Jul 31, 2023

johngmyers commented Aug 1, 2023 • edited Loading

cronik commented Aug 1, 2023

johngmyers commented Aug 11, 2023

johngmyers commented Sep 1, 2023

cronik commented Sep 2, 2023

johngmyers commented Sep 2, 2023

johngmyers commented Sep 3, 2023

johngmyers left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johngmyers commented Sep 3, 2023

johngmyers commented Sep 3, 2023

johngmyers commented Sep 3, 2023

johngmyers commented Sep 15, 2023

k8s-ci-robot commented Sep 15, 2023

max-blue commented Nov 1, 2023 • edited Loading

leonardocaylent commented Feb 9, 2024

cronik commented Feb 17, 2024 • edited Loading

leonardocaylent commented Feb 18, 2024

cronik commented Jun 28, 2023 •

edited

Loading

johngmyers commented Aug 1, 2023 •

edited

Loading

max-blue commented Nov 1, 2023 •

edited

Loading

cronik commented Feb 17, 2024 •

edited

Loading