Skip to content

Commit a974a07

Browse files
blakepetterssonblakepettersson
authored
fix: do not null fields if Argo CD API doesn't return fields (#701)
* fix: do not null fields if Argo CD API doesn't return fields Potentially fixes #697. Signed-off-by: Blake Pettersson <[email protected]> * test: add newline Signed-off-by: Blake Pettersson <[email protected]> * refactor: simplify Just do the conversions directly without any checks Signed-off-by: Blake Pettersson <[email protected]> * test: check for contains instead of a full string equals 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]> * fix: modify state instead of creating a new model 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]> --------- Signed-off-by: Blake Pettersson <[email protected]>
1 parent 26accfd commit a974a07

File tree

5 files changed

+642
-99
lines changed

5 files changed

+642
-99
lines changed

‎internal/provider/model_repository.go

Lines changed: 18 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -177,90 +177,48 @@ func (m *repositoryModel) toAPIModel() (*v1alpha1.Repository, error) {
177177
return repo, nil
178178
}
179179

180-
func newRepositoryModel(repo *v1alpha1.Repository) *repositoryModel {
181-
model := &repositoryModel{
182-
ID: types.StringValue(repo.Repo),
183-
Repo: types.StringValue(repo.Repo),
184-
EnableLFS: types.BoolValue(repo.EnableLFS),
185-
EnableOCI: types.BoolValue(repo.EnableOCI),
186-
Insecure: types.BoolValue(repo.Insecure),
187-
InheritedCreds: types.BoolValue(repo.InheritedCreds),
188-
}
189-
190-
// Handle connection state status
191-
if repo.ConnectionState.Status != "" {
192-
model.ConnectionStateStatus = types.StringValue(repo.ConnectionState.Status)
193-
} else {
194-
model.ConnectionStateStatus = types.StringNull()
195-
}
180+
func (m *repositoryModel) updateFromAPI(repo *v1alpha1.Repository) *repositoryModel {
181+
m.ID = types.StringValue(repo.Repo)
182+
m.Repo = types.StringValue(repo.Repo)
183+
m.Type = types.StringValue(repo.Type)
184+
m.EnableLFS = types.BoolValue(repo.EnableLFS)
185+
m.EnableOCI = types.BoolValue(repo.EnableOCI)
186+
m.Insecure = types.BoolValue(repo.Insecure)
187+
m.InheritedCreds = types.BoolValue(repo.InheritedCreds)
196188

197-
// Set string fields to null if empty, otherwise use the value
198189
if repo.Name != "" {
199-
model.Name = types.StringValue(repo.Name)
200-
} else {
201-
model.Name = types.StringNull()
190+
m.Name = types.StringValue(repo.Name)
202191
}
203192

204-
if repo.Type != "" {
205-
model.Type = types.StringValue(repo.Type)
206-
} else {
207-
model.Type = types.StringNull()
193+
// Handle connection state status
194+
if repo.ConnectionState.Status != "" {
195+
m.ConnectionStateStatus = types.StringValue(repo.ConnectionState.Status)
208196
}
209197

210198
if repo.Project != "" {
211-
model.Project = types.StringValue(repo.Project)
212-
} else {
213-
model.Project = types.StringNull()
199+
m.Project = types.StringValue(repo.Project)
214200
}
215201

216202
// Handle username based on inheritance
217203
if !repo.InheritedCreds {
218204
if repo.Username != "" {
219-
model.Username = types.StringValue(repo.Username)
220-
} else {
221-
model.Username = types.StringNull()
205+
m.Username = types.StringValue(repo.Username)
222206
}
223-
} else {
224-
model.Username = types.StringNull()
225207
}
226208

227209
if repo.GitHubAppEnterpriseBaseURL != "" {
228-
model.GitHubAppEnterpriseBaseURL = types.StringValue(repo.GitHubAppEnterpriseBaseURL)
229-
} else {
230-
model.GitHubAppEnterpriseBaseURL = types.StringNull()
210+
m.GitHubAppEnterpriseBaseURL = types.StringValue(repo.GitHubAppEnterpriseBaseURL)
231211
}
232212

233213
// Handle GitHub App ID conversion
234214
if repo.GithubAppId > 0 {
235-
model.GitHubAppID = types.StringValue(strconv.FormatInt(repo.GithubAppId, 10))
236-
} else {
237-
model.GitHubAppID = types.StringNull()
215+
m.GitHubAppID = types.StringValue(strconv.FormatInt(repo.GithubAppId, 10))
238216
}
239217

240218
// Handle GitHub App Installation ID conversion
241219
if repo.GithubAppInstallationId > 0 {
242-
model.GitHubAppInstallationID = types.StringValue(strconv.FormatInt(repo.GithubAppInstallationId, 10))
243-
} else {
244-
model.GitHubAppInstallationID = types.StringNull()
245-
}
246-
247-
// Handle credentials based on inheritance
248-
if !repo.InheritedCreds {
249-
if repo.TLSClientCertData != "" {
250-
model.TLSClientCertData = types.StringValue(repo.TLSClientCertData)
251-
} else {
252-
model.TLSClientCertData = types.StringNull()
253-
}
254-
} else {
255-
model.TLSClientCertData = types.StringNull()
220+
m.GitHubAppInstallationID = types.StringValue(strconv.FormatInt(repo.GithubAppInstallationId, 10))
256221
}
257222

258-
// Note: The ArgoCD API does not return sensitive fields (password, ssh_private_key, tls_client_cert_key,
259-
// githubapp_private_key), so they remain as configured in Terraform state
260-
model.Password = types.StringNull()
261-
model.SSHPrivateKey = types.StringNull()
262-
model.TLSClientCertKey = types.StringNull()
263-
model.GitHubAppPrivateKey = types.StringNull()
264-
265-
return model
223+
return m
266224
}

‎internal/provider/resource_repository.go

Lines changed: 16 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -129,17 +129,8 @@ func (r *repositoryResource) Create(ctx context.Context, req resource.CreateRequ
129129

130130
tflog.Trace(ctx, fmt.Sprintf("created repository %s", createdRepo.Repo))
131131

132-
// Update the model with the created repository data
133-
result := newRepositoryModel(createdRepo)
134-
135-
// Preserve sensitive fields from the original configuration
136-
result.Password = data.Password
137-
result.SSHPrivateKey = data.SSHPrivateKey
138-
result.TLSClientCertKey = data.TLSClientCertKey
139-
result.GitHubAppPrivateKey = data.GitHubAppPrivateKey
140-
141132
// Save data into Terraform state
142-
resp.Diagnostics.Append(resp.State.Set(ctx, result)...)
133+
resp.Diagnostics.Append(resp.State.Set(ctx, data.updateFromAPI(repo))...)
143134

144135
// Perform a read to get the latest state with connection status
145136
if !resp.Diagnostics.HasError() {
@@ -178,17 +169,8 @@ func (r *repositoryResource) Read(ctx context.Context, req resource.ReadRequest,
178169
return
179170
}
180171

181-
// Update the model with the read repository data
182-
result := newRepositoryModel(repo)
183-
184-
// Preserve sensitive fields from the original state
185-
result.Password = data.Password
186-
result.SSHPrivateKey = data.SSHPrivateKey
187-
result.TLSClientCertKey = data.TLSClientCertKey
188-
result.GitHubAppPrivateKey = data.GitHubAppPrivateKey
189-
190172
// Save updated data into Terraform state
191-
resp.Diagnostics.Append(resp.State.Set(ctx, result)...)
173+
resp.Diagnostics.Append(resp.State.Set(ctx, data.updateFromAPI(repo))...)
192174
}
193175

194176
func (r *repositoryResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) {
@@ -214,14 +196,13 @@ func (r *repositoryResource) Update(ctx context.Context, req resource.UpdateRequ
214196

215197
// Update repository
216198
sync.RepositoryMutex.Lock()
199+
defer sync.RepositoryMutex.Unlock()
217200

218201
updatedRepo, err := r.si.RepositoryClient.UpdateRepository(
219202
ctx,
220203
&repository.RepoUpdateRequest{Repo: repo},
221204
)
222205

223-
sync.RepositoryMutex.Unlock()
224-
225206
if err != nil {
226207
resp.Diagnostics.Append(diagnostics.ArgoCDAPIError("update", "repository", repo.Repo, err)...)
227208
return
@@ -243,17 +224,16 @@ func (r *repositoryResource) Update(ctx context.Context, req resource.UpdateRequ
243224

244225
tflog.Trace(ctx, fmt.Sprintf("updated repository %s", updatedRepo.Repo))
245226

246-
// Update the model with the updated repository data
247-
result := newRepositoryModel(updatedRepo)
248-
249-
// Preserve sensitive fields from the original configuration
250-
result.Password = data.Password
251-
result.SSHPrivateKey = data.SSHPrivateKey
252-
result.TLSClientCertKey = data.TLSClientCertKey
253-
result.GitHubAppPrivateKey = data.GitHubAppPrivateKey
254-
255227
// Save updated data into Terraform state
256-
resp.Diagnostics.Append(resp.State.Set(ctx, result)...)
228+
resp.Diagnostics.Append(resp.State.Set(ctx, data)...)
229+
230+
// Perform a read to get the latest state
231+
if !resp.Diagnostics.HasError() {
232+
readResp := &resource.ReadResponse{State: resp.State, Diagnostics: resp.Diagnostics}
233+
r.Read(ctx, resource.ReadRequest{State: resp.State}, readResp)
234+
resp.Diagnostics = readResp.Diagnostics
235+
resp.State = readResp.State
236+
}
257237
}
258238

259239
func (r *repositoryResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) {
@@ -272,6 +252,7 @@ func (r *repositoryResource) Delete(ctx context.Context, req resource.DeleteRequ
272252

273253
// Delete repository
274254
sync.RepositoryMutex.Lock()
255+
defer sync.RepositoryMutex.Unlock()
275256

276257
_, err := r.si.RepositoryClient.DeleteRepository(
277258
ctx,
@@ -281,8 +262,6 @@ func (r *repositoryResource) Delete(ctx context.Context, req resource.DeleteRequ
281262
},
282263
)
283264

284-
sync.RepositoryMutex.Unlock()
285-
286265
if err != nil {
287266
if !strings.Contains(err.Error(), "NotFound") {
288267
resp.Diagnostics.Append(diagnostics.ArgoCDAPIError("delete", "repository", data.ID.ValueString(), err)...)
@@ -310,11 +289,9 @@ func (r *repositoryResource) readRepository(ctx context.Context, repoURL, projec
310289
var finalRepo *v1alpha1.Repository
311290

312291
if repos != nil {
313-
if len(repos.Items) >= 1 {
314-
for _, repo := range repos.Items {
315-
if repo.Repo == repoURL {
316-
finalRepo = repo
317-
}
292+
for _, repo := range repos.Items {
293+
if repo.Repo == repoURL {
294+
finalRepo = repo
318295
}
319296
}
320297
}

‎internal/provider/resource_repository_certificate_test.go

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
1414
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
1515
"github.com/stretchr/testify/assert"
16+
"github.com/stretchr/testify/require"
1617
)
1718

1819
func TestAccArgoCDRepositoryCertificatesSSH(t *testing.T) {
@@ -494,3 +495,134 @@ func getSshKeysForHost(host string) ([]string, error) {
494495

495496
return subTypesKeys, nil
496497
}
498+
499+
// TestAccArgoCDRepositoryCertificate_SSHConsistency tests consistency of SSH certificate fields
500+
func TestAccArgoCDRepositoryCertificate_SSHConsistency(t *testing.T) {
501+
serverName := acctest.RandomWithPrefix("ssh-test")
502+
503+
config := testAccArgoCDRepositoryCertificatesSSH(
504+
serverName,
505+
"ssh-rsa",
506+
"AAAAB3NzaC1yc2EAAAABIwAAAQEAq2A7hRGmdnm9tUDbO9IDSwBK6TbQa+PXYPCPy6rbTrTtw7PHkccKrpp0yVhp5HdEIcKr6pLlVDBfOLX9QUsyCOV0wzfjIJNlGEYsdlLJizHhbn2mUjvSAHQqZETYP81eFzLQNnPHt4EVVUh7VfDESU84KezmD5QlWpXLmvU31/yMf+Se8xhHTvKSCZIFImWwoG6mbUoWf9nzpIoaSjB+weqqUUmpaaasXVal72J+UX2B+2RPW3RcT0eOzQgqlJL3RKrTJvdsjE3JEAvGq3lGHSZXy28G3skua2SmVi/w4yCE6gbODqnTWlg7+wC604ydGXA8VJiS5ap43JXiUFFAaQ==",
507+
)
508+
509+
resource.Test(t, resource.TestCase{
510+
PreCheck: func() { testAccPreCheck(t) },
511+
ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
512+
Steps: []resource.TestStep{
513+
{
514+
Config: config,
515+
Check: resource.ComposeAggregateTestCheckFunc(
516+
resource.TestCheckResourceAttr(
517+
"argocd_repository_certificate.simple",
518+
"ssh.0.server_name",
519+
serverName,
520+
),
521+
resource.TestCheckResourceAttr(
522+
"argocd_repository_certificate.simple",
523+
"ssh.0.cert_subtype",
524+
"ssh-rsa",
525+
),
526+
resource.TestCheckResourceAttrSet(
527+
"argocd_repository_certificate.simple",
528+
"ssh.0.cert_info",
529+
),
530+
),
531+
},
532+
{
533+
// Apply the same configuration again to test for consistency
534+
Config: config,
535+
Check: resource.ComposeAggregateTestCheckFunc(
536+
resource.TestCheckResourceAttr(
537+
"argocd_repository_certificate.simple",
538+
"ssh.0.server_name",
539+
serverName,
540+
),
541+
resource.TestCheckResourceAttr(
542+
"argocd_repository_certificate.simple",
543+
"ssh.0.cert_subtype",
544+
"ssh-rsa",
545+
),
546+
resource.TestCheckResourceAttrSet(
547+
"argocd_repository_certificate.simple",
548+
"ssh.0.cert_info",
549+
),
550+
),
551+
},
552+
},
553+
})
554+
}
555+
556+
// TestAccArgoCDRepositoryCertificate_HTTPSConsistency tests consistency of HTTPS certificate fields
557+
func TestAccArgoCDRepositoryCertificate_HTTPSConsistency(t *testing.T) {
558+
serverName := acctest.RandomWithPrefix("https-test")
559+
certData := "-----BEGIN CERTIFICATE-----\nMIIFajCCBPCgAwIBAgIQBRiaVOvox+kD4KsNklVF3jAKBggqhkjOPQQDAzBWMQsw\nCQYDVQQGEwJVUzEVMBMGA1UEChMMRGlnaUNlcnQgSW5jMTAwLgYDVQQDEydEaWdp\nQ2VydCBUTFMgSHlicmlkIEVDQyBTSEEzODQgMjAyMCBDQTEwHhcNMjIwMzE1MDAw\nMDAwWhcNMjMwMzE1MjM1OTU5WjBmMQswCQYDVQQGEwJVUzETMBEGA1UECBMKQ2Fs\naWZvcm5pYTEWMBQGA1UEBxMNU2FuIEZyYW5jaXNjbzEVMBMGA1UEChMMR2l0SHVi\nLCBJbmMuMRMwEQYDVQQDEwpnaXRodWIuY29tMFkwEwYHKoZIzj0CAQYIKoZIzj0D\nAQcDQgAESrCTcYUh7GI/y3TARsjnANwnSjJLitVRgwgRI1JlxZ1kdZQQn5ltP3v7\nKTtYuDdUeEu3PRx3fpDdu2cjMlyA0aOCA44wggOKMB8GA1UdIwQYMBaAFAq8CCkX\njKU5bXoOzjPHLrPt+8N6MB0GA1UdDgQWBBR4qnLGcWloFLVZsZ6LbitAh0I7HjAl\nBgNVHREEHjAcggpnaXRodWIuY29tgg53d3cuZ2l0aHViLmNvbTAOBgNVHQ8BAf8E\nBAMCB4AwHQYDVR0lBBYwFAYIKwYBBQUHAwEGCCsGAQUFBwMCMIGbBgNVHR8EgZMw\ngZAwRqBEoEKGQGh0dHA6Ly9jcmwzLmRpZ2ljZXJ0LmNvbS9EaWdpQ2VydFRMU0h5\nYnJpZEVDQ1NIQTM4NDIwMjBDQTEtMS5jcmwwRqBEoEKGQGh0dHA6Ly9jcmw0LmRp\nZ2ljZXJ0LmNvbS9EaWdpQ2VydFRMU0h5YnJpZEVDQ1NIQTM4NDIwMjBDQTEtMS5j\ncmwwPgYDVR0gBDcwNTAzBgZngQwBAgIwKTAnBggrBgEFBQcCARYbaHR0cDovL3d3\ndy5kaWdpY2VydC5jb20vQ1BTMIGFBggrBgEFBQcBAQR5MHcwJAYIKwYBBQUHMAGG\nGGh0dHA6Ly9vY3NwLmRpZ2ljZXJ0LmNvbTBPBggrBgEFBQcwAoZDaHR0cDovL2Nh\nY2VydHMuZGlnaWNlcnQuY29tL0RpZ2lDZXJ0VExTSHlicmlkRUNDU0hBMzg0MjAy\nMENBMS0xLmNydDAJBgNVHRMEAjAAMIIBfwYKKwYBBAHWeQIEAgSCAW8EggFrAWkA\ndgCt9776fP8QyIudPZwePhhqtGcpXc+xDCTKhYY069yCigAAAX+Oi8SRAAAEAwBH\nMEUCIAR9cNnvYkZeKs9JElpeXwztYB2yLhtc8bB0rY2ke98nAiEAjiML8HZ7aeVE\nP/DkUltwIS4c73VVrG9JguoRrII7gWMAdwA1zxkbv7FsV78PrUxtQsu7ticgJlHq\nP+Eq76gDwzvWTAAAAX+Oi8R7AAAEAwBIMEYCIQDNckqvBhup7GpANMf0WPueytL8\nu/PBaIAObzNZeNMpOgIhAMjfEtE6AJ2fTjYCFh/BNVKk1mkTwBTavJlGmWomQyaB\nAHYAs3N3B+GEUPhjhtYFqdwRCUp5LbFnDAuH3PADDnk2pZoAAAF/jovErAAABAMA\nRzBFAiEA9Uj5Ed/XjQpj/MxQRQjzG0UFQLmgWlc73nnt3CJ7vskCICqHfBKlDz7R\nEHdV5Vk8bLMBW1Q6S7Ga2SbFuoVXs6zFMAoGCCqGSM49BAMDA2gAMGUCMCiVhqft\n7L/stBmv1XqSRNfE/jG/AqKIbmjGTocNbuQ7kt1Cs7kRg+b3b3C9Ipu5FQIxAM7c\ntGKrYDGt0pH8iF6rzbp9Q4HQXMZXkNxg+brjWxnaOVGTDNwNH7048+s/hT9bUQ==\n-----END CERTIFICATE-----"
560+
561+
config := testAccArgoCDRepositoryCertificateHttps(serverName, certData)
562+
563+
resource.Test(t, resource.TestCase{
564+
PreCheck: func() { testAccPreCheck(t) },
565+
ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
566+
Steps: []resource.TestStep{
567+
{
568+
Config: config,
569+
Check: resource.ComposeAggregateTestCheckFunc(
570+
resource.TestCheckResourceAttr(
571+
"argocd_repository_certificate.simple",
572+
"https.0.server_name",
573+
serverName,
574+
),
575+
resource.TestCheckResourceAttrWith(
576+
"argocd_repository_certificate.simple",
577+
"https.0.cert_data",
578+
func(value string) error {
579+
// Not yet sure why the impl is suffixing with newline. Adding a newline only makes the test fail,
580+
// since it'll add yet another newline.
581+
require.Contains(t, value, certData)
582+
return nil
583+
},
584+
),
585+
resource.TestCheckResourceAttr(
586+
"argocd_repository_certificate.simple",
587+
"https.0.cert_subtype",
588+
"ecdsa",
589+
),
590+
resource.TestCheckResourceAttrSet(
591+
"argocd_repository_certificate.simple",
592+
"https.0.cert_info",
593+
),
594+
),
595+
},
596+
{
597+
// Apply the same configuration again to test for consistency
598+
Config: config,
599+
Check: resource.ComposeAggregateTestCheckFunc(
600+
resource.TestCheckResourceAttr(
601+
"argocd_repository_certificate.simple",
602+
"https.0.server_name",
603+
serverName,
604+
),
605+
resource.TestCheckResourceAttrWith(
606+
"argocd_repository_certificate.simple",
607+
"https.0.cert_data",
608+
func(value string) error {
609+
// Not yet sure why the impl is suffixing with newline. Adding a newline only makes the test fail,
610+
// since it'll add yet another newline.
611+
require.Contains(t, value, certData)
612+
return nil
613+
},
614+
),
615+
resource.TestCheckResourceAttr(
616+
"argocd_repository_certificate.simple",
617+
"https.0.cert_subtype",
618+
"ecdsa",
619+
),
620+
resource.TestCheckResourceAttrSet(
621+
"argocd_repository_certificate.simple",
622+
"https.0.cert_info",
623+
),
624+
),
625+
},
626+
},
627+
})
628+
}

0 commit comments

Comments
 (0)