Skip to content

fix: do not null fields if Argo CD API doesn't return fields #701

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Aug 12, 2025
Merged

fix: do not null fields if Argo CD API doesn't return fields #701

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b10f2dd
fix: do not null fields if Argo CD API doesn't return fields
blakepettersson Aug 3, 2025
49f0e16
test: add newline
blakepettersson Aug 4, 2025
6840140
refactor: simplify
blakepettersson Aug 4, 2025
e574a83
test: check for contains instead of a full string equals
blakepettersson Aug 4, 2025
82d9c3d
fix: modify state instead of creating a new model
blakepettersson Aug 7, 2025
83c9948
Merge branch 'main' into fix/inconsistent-state-for-repositories
blakepettersson Aug 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 18 additions & 60 deletions internal/provider/model_repository.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,90 +177,48 @@ func (m *repositoryModel) toAPIModel() (*v1alpha1.Repository, error) {
return repo, nil
}

func newRepositoryModel(repo *v1alpha1.Repository) *repositoryModel {
model := &repositoryModel{
ID: types.StringValue(repo.Repo),
Repo: types.StringValue(repo.Repo),
EnableLFS: types.BoolValue(repo.EnableLFS),
EnableOCI: types.BoolValue(repo.EnableOCI),
Insecure: types.BoolValue(repo.Insecure),
InheritedCreds: types.BoolValue(repo.InheritedCreds),
}

// Handle connection state status
if repo.ConnectionState.Status != "" {
model.ConnectionStateStatus = types.StringValue(repo.ConnectionState.Status)
} else {
model.ConnectionStateStatus = types.StringNull()
}
func (m *repositoryModel) updateFromAPI(repo *v1alpha1.Repository) *repositoryModel {
m.ID = types.StringValue(repo.Repo)
m.Repo = types.StringValue(repo.Repo)
m.Type = types.StringValue(repo.Type)
m.EnableLFS = types.BoolValue(repo.EnableLFS)
m.EnableOCI = types.BoolValue(repo.EnableOCI)
m.Insecure = types.BoolValue(repo.Insecure)
m.InheritedCreds = types.BoolValue(repo.InheritedCreds)

// Set string fields to null if empty, otherwise use the value
if repo.Name != "" {
model.Name = types.StringValue(repo.Name)
} else {
model.Name = types.StringNull()
m.Name = types.StringValue(repo.Name)
}

if repo.Type != "" {
model.Type = types.StringValue(repo.Type)
} else {
model.Type = types.StringNull()
// Handle connection state status
if repo.ConnectionState.Status != "" {
m.ConnectionStateStatus = types.StringValue(repo.ConnectionState.Status)
}

if repo.Project != "" {
model.Project = types.StringValue(repo.Project)
} else {
model.Project = types.StringNull()
m.Project = types.StringValue(repo.Project)
}

// Handle username based on inheritance
if !repo.InheritedCreds {
if repo.Username != "" {
model.Username = types.StringValue(repo.Username)
} else {
model.Username = types.StringNull()
m.Username = types.StringValue(repo.Username)
}
} else {
model.Username = types.StringNull()
}

if repo.GitHubAppEnterpriseBaseURL != "" {
model.GitHubAppEnterpriseBaseURL = types.StringValue(repo.GitHubAppEnterpriseBaseURL)
} else {
model.GitHubAppEnterpriseBaseURL = types.StringNull()
m.GitHubAppEnterpriseBaseURL = types.StringValue(repo.GitHubAppEnterpriseBaseURL)
}

// Handle GitHub App ID conversion
if repo.GithubAppId > 0 {
model.GitHubAppID = types.StringValue(strconv.FormatInt(repo.GithubAppId, 10))
} else {
model.GitHubAppID = types.StringNull()
m.GitHubAppID = types.StringValue(strconv.FormatInt(repo.GithubAppId, 10))
}

// Handle GitHub App Installation ID conversion
if repo.GithubAppInstallationId > 0 {
model.GitHubAppInstallationID = types.StringValue(strconv.FormatInt(repo.GithubAppInstallationId, 10))
} else {
model.GitHubAppInstallationID = types.StringNull()
}

// Handle credentials based on inheritance
if !repo.InheritedCreds {
if repo.TLSClientCertData != "" {
model.TLSClientCertData = types.StringValue(repo.TLSClientCertData)
} else {
model.TLSClientCertData = types.StringNull()
}
} else {
model.TLSClientCertData = types.StringNull()
m.GitHubAppInstallationID = types.StringValue(strconv.FormatInt(repo.GithubAppInstallationId, 10))
}

// Note: The ArgoCD API does not return sensitive fields (password, ssh_private_key, tls_client_cert_key,
// githubapp_private_key), so they remain as configured in Terraform state
model.Password = types.StringNull()
model.SSHPrivateKey = types.StringNull()
model.TLSClientCertKey = types.StringNull()
model.GitHubAppPrivateKey = types.StringNull()

return model
return m
}
55 changes: 16 additions & 39 deletions internal/provider/resource_repository.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,17 +129,8 @@ func (r *repositoryResource) Create(ctx context.Context, req resource.CreateRequ

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

// Update the model with the created repository data
result := newRepositoryModel(createdRepo)

// Preserve sensitive fields from the original configuration
result.Password = data.Password
result.SSHPrivateKey = data.SSHPrivateKey
result.TLSClientCertKey = data.TLSClientCertKey
result.GitHubAppPrivateKey = data.GitHubAppPrivateKey

// Save data into Terraform state
resp.Diagnostics.Append(resp.State.Set(ctx, result)...)
resp.Diagnostics.Append(resp.State.Set(ctx, data.updateFromAPI(repo))...)

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

// Update the model with the read repository data
result := newRepositoryModel(repo)

// Preserve sensitive fields from the original state
result.Password = data.Password
result.SSHPrivateKey = data.SSHPrivateKey
result.TLSClientCertKey = data.TLSClientCertKey
result.GitHubAppPrivateKey = data.GitHubAppPrivateKey

// Save updated data into Terraform state
resp.Diagnostics.Append(resp.State.Set(ctx, result)...)
resp.Diagnostics.Append(resp.State.Set(ctx, data.updateFromAPI(repo))...)
}

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

// Update repository
sync.RepositoryMutex.Lock()
defer sync.RepositoryMutex.Unlock()

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

sync.RepositoryMutex.Unlock()

if err != nil {
resp.Diagnostics.Append(diagnostics.ArgoCDAPIError("update", "repository", repo.Repo, err)...)
return
Expand All @@ -243,17 +224,16 @@ func (r *repositoryResource) Update(ctx context.Context, req resource.UpdateRequ

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

// Update the model with the updated repository data
result := newRepositoryModel(updatedRepo)

// Preserve sensitive fields from the original configuration
result.Password = data.Password
result.SSHPrivateKey = data.SSHPrivateKey
result.TLSClientCertKey = data.TLSClientCertKey
result.GitHubAppPrivateKey = data.GitHubAppPrivateKey

// Save updated data into Terraform state
resp.Diagnostics.Append(resp.State.Set(ctx, result)...)
resp.Diagnostics.Append(resp.State.Set(ctx, data)...)

// Perform a read to get the latest state
if !resp.Diagnostics.HasError() {
readResp := &resource.ReadResponse{State: resp.State, Diagnostics: resp.Diagnostics}
r.Read(ctx, resource.ReadRequest{State: resp.State}, readResp)
resp.Diagnostics = readResp.Diagnostics
resp.State = readResp.State
}
}

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

// Delete repository
sync.RepositoryMutex.Lock()
defer sync.RepositoryMutex.Unlock()

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

sync.RepositoryMutex.Unlock()

if err != nil {
if !strings.Contains(err.Error(), "NotFound") {
resp.Diagnostics.Append(diagnostics.ArgoCDAPIError("delete", "repository", data.ID.ValueString(), err)...)
Expand Down Expand Up @@ -310,11 +289,9 @@ func (r *repositoryResource) readRepository(ctx context.Context, repoURL, projec
var finalRepo *v1alpha1.Repository

if repos != nil {
if len(repos.Items) >= 1 {
for _, repo := range repos.Items {
if repo.Repo == repoURL {
finalRepo = repo
}
for _, repo := range repos.Items {
if repo.Repo == repoURL {
finalRepo = repo
}
}
}
Expand Down
132 changes: 132 additions & 0 deletions internal/provider/resource_repository_certificate_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

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

return subTypesKeys, nil
}

// TestAccArgoCDRepositoryCertificate_SSHConsistency tests consistency of SSH certificate fields
func TestAccArgoCDRepositoryCertificate_SSHConsistency(t *testing.T) {
serverName := acctest.RandomWithPrefix("ssh-test")

config := testAccArgoCDRepositoryCertificatesSSH(
serverName,
"ssh-rsa",
"AAAAB3NzaC1yc2EAAAABIwAAAQEAq2A7hRGmdnm9tUDbO9IDSwBK6TbQa+PXYPCPy6rbTrTtw7PHkccKrpp0yVhp5HdEIcKr6pLlVDBfOLX9QUsyCOV0wzfjIJNlGEYsdlLJizHhbn2mUjvSAHQqZETYP81eFzLQNnPHt4EVVUh7VfDESU84KezmD5QlWpXLmvU31/yMf+Se8xhHTvKSCZIFImWwoG6mbUoWf9nzpIoaSjB+weqqUUmpaaasXVal72J+UX2B+2RPW3RcT0eOzQgqlJL3RKrTJvdsjE3JEAvGq3lGHSZXy28G3skua2SmVi/w4yCE6gbODqnTWlg7+wC604ydGXA8VJiS5ap43JXiUFFAaQ==",
)

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
Steps: []resource.TestStep{
{
Config: config,
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr(
"argocd_repository_certificate.simple",
"ssh.0.server_name",
serverName,
),
resource.TestCheckResourceAttr(
"argocd_repository_certificate.simple",
"ssh.0.cert_subtype",
"ssh-rsa",
),
resource.TestCheckResourceAttrSet(
"argocd_repository_certificate.simple",
"ssh.0.cert_info",
),
),
},
{
// Apply the same configuration again to test for consistency
Config: config,
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr(
"argocd_repository_certificate.simple",
"ssh.0.server_name",
serverName,
),
resource.TestCheckResourceAttr(
"argocd_repository_certificate.simple",
"ssh.0.cert_subtype",
"ssh-rsa",
),
resource.TestCheckResourceAttrSet(
"argocd_repository_certificate.simple",
"ssh.0.cert_info",
),
),
},
},
})
}

// TestAccArgoCDRepositoryCertificate_HTTPSConsistency tests consistency of HTTPS certificate fields
func TestAccArgoCDRepositoryCertificate_HTTPSConsistency(t *testing.T) {
serverName := acctest.RandomWithPrefix("https-test")
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-----"

config := testAccArgoCDRepositoryCertificateHttps(serverName, certData)

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
Steps: []resource.TestStep{
{
Config: config,
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr(
"argocd_repository_certificate.simple",
"https.0.server_name",
serverName,
),
resource.TestCheckResourceAttrWith(
"argocd_repository_certificate.simple",
"https.0.cert_data",
func(value string) error {
// Not yet sure why the impl is suffixing with newline. Adding a newline only makes the test fail,
// since it'll add yet another newline.
require.Contains(t, value, certData)
return nil
},
),
resource.TestCheckResourceAttr(
"argocd_repository_certificate.simple",
"https.0.cert_subtype",
"ecdsa",
),
resource.TestCheckResourceAttrSet(
"argocd_repository_certificate.simple",
"https.0.cert_info",
),
),
},
{
// Apply the same configuration again to test for consistency
Config: config,
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr(
"argocd_repository_certificate.simple",
"https.0.server_name",
serverName,
),
resource.TestCheckResourceAttrWith(
"argocd_repository_certificate.simple",
"https.0.cert_data",
func(value string) error {
// Not yet sure why the impl is suffixing with newline. Adding a newline only makes the test fail,
// since it'll add yet another newline.
require.Contains(t, value, certData)
return nil
},
),
resource.TestCheckResourceAttr(
"argocd_repository_certificate.simple",
"https.0.cert_subtype",
"ecdsa",
),
resource.TestCheckResourceAttrSet(
"argocd_repository_certificate.simple",
"https.0.cert_info",
),
),
},
},
})
}
Loading