fix: do not null fields if Argo CD API doesn't return fields #701
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
blakepettersson
requested review from
oboukili,
onematchfox,
the-technat and
mkilchhofer
as code owners
Potentially fixes argoproj-labs#697. Signed-off-by: Blake Pettersson <[email protected]>
blakepettersson
force-pushed
the
fix/inconsistent-state-for-repositories
branch
from
c36ab5a to
b10f2dd
Compare
Signed-off-by: Blake Pettersson <[email protected]>
Just do the conversions directly without any checks Signed-off-by: Blake Pettersson <[email protected]>
blakepettersson
force-pushed
the
fix/inconsistent-state-for-repositories
branch
from
5eec0de to
6840140
Compare
For some reason this always suffixes a newline. If I add a newline, then it adds yet another newline, etc etc. Signed-off-by: Blake Pettersson <[email protected]>
onematchfox
reviewed
Aug 4, 2025
Comment on lines
140
to
145
| result.Name = data.Name | ||
| result.Username = data.Username | ||
| result.GitHubAppID = data.GitHubAppID | ||
| result.GitHubAppInstallationID = data.GitHubAppInstallationID | ||
| result.GitHubAppEnterpriseBaseURL = data.GitHubAppEnterpriseBaseURL | ||
| result.TLSClientCertData = data.TLSClientCertData |
There was a problem hiding this comment.
Just checking... Are all of these treated as sensitive by ArgoCD? Surely the ArgoCD API at least returns Name? I would also think it potentially returns Username, GitHubAppEnterpriseBaseURL and TLSClientCertData.
To stay more true to the former implementation (pre terraform-plugin-framework) we should just load the state, and make any prerequisite modifications from the API if necessary. This also resolves the issues coming from the tf-plugin-framework migration. Signed-off-by: Blake Pettersson <[email protected]>
blakepettersson
force-pushed
the
fix/inconsistent-state-for-repositories
branch
from
979827c to
82d9c3d
Compare
the-technat
approved these changes
Aug 11, 2025
blakepettersson
added a commit
to blakepettersson/terraform-provider-argocd
that referenced
this pull request
Aug 13, 2025
Similar to what has been done in argoproj-labs#701, add consistency tests to verify that there won't be any inconsistent state after doing a few applies. Signed-off-by: Blake Pettersson <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Potentially fixes #697.