Closed SimonKienzler closed 6 months ago
@PatrickKoss just FYI, we tested a pre-build of the code in this PR in our environment, and both the Key and the Token flow work as expected. For the token flow, it's almost a drop-in replacement, the upgrade path only consists of changing AUTH_TOKEN
to STACKIT_SERVICE_ACCOUNT_TOKEN
in the Deployment's env
settings, as demonstrated in the updated README.
@PatrickKoss PTAL 😊
BTW, what's up with the failing check for a semantic PR title? I based the title on https://github.com/stackitcloud/external-dns-stackit-webhook/blob/main/.github/semantic.yml and thought it fulfills the requirements 🤔
@PatrickKoss PTAL 😊
BTW, what's up with the failing check for a semantic PR title? I based the title on https://github.com/stackitcloud/external-dns-stackit-webhook/blob/main/.github/semantic.yml and thought it fulfills the requirements 🤔
its somehow broken :D That is something we need to fix #soon
This PR replaces the previously used https://github.com/stackitcloud/stackit-dns-api-client-go with the official https://github.com/stackitcloud/stackit-sdk-go in order to
The largest portion of the changes in terms of LoC are struct type switches, which should not be too much of an issue. However, as I was previously unfamiliar with the code base, I'd like to point reviewers to the following changes, which might be worth a closer look:
NewStackitDNSProvider()
function now takes variadic STACKIT config options as the last argument in order to override the client and authentication config for the unit tests.token
andbaseURL
fields from the provider'sConfig
struct, as these values are only relevant to the STACKIT SDK, which gets them via the variadic config options now.I also removed the requiredauth-token
flag/env var setting, as the SDK client itself throws a panic when it is not properly set up with either a token or a key. Double-wrapping this with a check in the root command seemed unnecessary to me.fetchRecords()
andfetchZones()
passes all unit tests, but might be worth a rigorous review nonetheless.getApplyChangesBasicTestCases()
was removed. This test case had a comment saying swagger client does not return an error when the response is invalid json, which is now obsolete. The behavior actually differs now betweenCREATE
andDELETE
/UPDATE
, as theCreate()
call actually tries to parse the response. Therefore, this test case is of little use. Please advise if you prefer this to be handled differently., and rather added a link to the SDK auth options in order to not duplicate soon-to-be outdated information.