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

Remove ManagedZone API #203

Merged
merged 1 commit into from
Aug 19, 2024
Merged

Remove ManagedZone API #203

merged 1 commit into from
Aug 19, 2024

Conversation

mikenairn
Copy link
Member

@mikenairn mikenairn commented Jul 31, 2024

Part of : #166

Removes the ManagedZone API and all related code and docs.

The DNSRecord resource no longer has a reference to a ManagedZone, instead a new providerRef field is added which references a single dns provider secret. The contents of this secret are identical to what was previously used by the ManagedZone.dnsProviderSecretRef.

apiVersion: v1
kind: Secret
metadata:
  name: myawsroute53
type: kuadrant.io/aws
stringData:
 AWS_ACCESS_KEY_ID: "xyz"
 AWS_SECRET_ACCESS_KEY: "12345"

The secrets do not contain information about what zone a record will be published into, instead a zone in the dns provider will be assigned to a DNSRecord via lookup on it's first reconcile. What zone will be used, if any, is determined by the zones available via the credentials in the dns provider secret. Once the zone is assigned the information of the zone used is stored in the DNSRecord status and used by all requests for this record to the provider thereafter:

"zoneDomainName": "mn.example.com",
"zoneID": "/hostedzone/12345abcd"

Note: CI-E2E / E2E Test Suite (pull_request_target) CI job will fail until it's merged. The e2e test is being tested as a branch job for now see.

ToDo:

  • Handle multiple zones with the same dns name when finding a suitable zone to assign to a record

@mikenairn mikenairn force-pushed the remove_managed_zone_api branch 5 times, most recently from 8193e81 to 777df87 Compare July 31, 2024 16:35
@mikenairn mikenairn force-pushed the remove_managed_zone_api branch 4 times, most recently from e79ebbc to d32ba59 Compare August 12, 2024 10:08
@@ -5,6 +5,7 @@ on:
branches:
- main
- "release-*"
- remove_managed_zone_api
Copy link
Member Author

Choose a reason for hiding this comment

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

ToDo remove before merge

newTag: latest
newTag: remove_managed_zone_api

#ToDo mnairn: Revert tag before merge
Copy link
Member Author

Choose a reason for hiding this comment

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

ToDo revert before merge

StringData: pb.strDataItems,
Type: pb.secretType,
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

I added this provider builder as a public API so it could be used in both the tests here and in kuadrant operator. Unsure of the best location for it, I'm not really sure if an API like this should be inside the api/v1alpha1 package, or just be outside and always work with the latest version of the APIs.

@mikenairn mikenairn force-pushed the remove_managed_zone_api branch from d32ba59 to d81a4a9 Compare August 12, 2024 11:04
@mikenairn mikenairn marked this pull request as ready for review August 12, 2024 11:04
Makefile Outdated Show resolved Hide resolved
return ctrl.Result{RequeueAfter: randomizedValidationRequeue}, nil
}
} else {
logger.Info(fmt.Sprintf("unable to perform cleanup of dns provider for zone status id: '%s', dominaName: '%s'",
Copy link
Collaborator

Choose a reason for hiding this comment

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

is it possible to give more info than this? The way I read it, this can happen if we failed to set a zid or domain inside the zone. As this is a delete also makes me wonder if this could happen. I guess if a record was created and there was a bad credential and then you tried to delete it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, if the record is created and for whatever reason it isn't able to assign a zone e.g. invalid credentials, no zone that matches the root host etc... we shouldn't try and cleanup when the record is deleted since it won't have been able to create anything in the first place.

@mikenairn mikenairn changed the title WIP chore: Remove ManagedZone API chore: Remove ManagedZone API Aug 12, 2024
@mikenairn mikenairn changed the title chore: Remove ManagedZone API Remove ManagedZone API Aug 12, 2024
@mikenairn mikenairn force-pushed the remove_managed_zone_api branch 5 times, most recently from 46cdd6c to 2b835e4 Compare August 14, 2024 09:05
Removes the ManagedZone API and all related code and docs.

Signed-off-by: Michael Nairn <[email protected]>

Signed-off-by: craig <[email protected]>
@maleck13 maleck13 force-pushed the remove_managed_zone_api branch from 2b835e4 to dd7e59b Compare August 19, 2024 08:42
@maleck13 maleck13 merged commit 8a6812d into main Aug 19, 2024
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants