fix: validate resolved OIDC config after secret substitution (#29545)
Project: argoproj/argo-cd | Language: Go | Concepts: error-handling, configuration-management, testing-strategy | Difficulty: mid
Link: https://github.com/argoproj/argo-cd/pull/29555
The problem
When Argo CD reloads its settings, it validates the OIDC (OpenID Connect) configuration before substituting any referenced Kubernetes Secrets. This means that if the config uses a secret reference (e.g., issuer: $my-secret:issuer), validation fails—even if the referenced secret contains a valid value. As a result, users see a spurious "Failed to validate OIDC config" warning, even though SSO works at runtime.
Why it's non-trivial
A naive fix might simply skip validation or validate the config as-is, but this would either allow invalid configurations to go unnoticed or continue to reject valid secret references. The correct fix must resolve all secret references before validation, so that both literal and secret-backed values are checked correctly. It must also ensure that invalid values stored in secrets are caught.
The approach
1. Change validation to operate on resolved config (with secrets substituted):
- if err := ValidateOIDCConfig(settings.OIDCConfigRAW); err != nil {
+ if err := validateOIDCConfigWithSecrets(settings.OIDCConfigRAW, settings.Secrets); err != nil {
log.Warnf("Failed to validate OIDC config: %v", err)
}
This replaces the old validation call with a new helper that substitutes secrets before validating. This ensures that validation sees the actual values, not the secret references.
2. Implement helper to substitute secrets and validate:
func validateOIDCConfigWithSecrets(configStr string, secrets map[string]string) error {
configMap := map[string]any{}
if err := yaml.Unmarshal([]byte(configStr), &configMap); err != nil {
return err
}
configMap = ReplaceMapSecrets(configMap, secrets)
resolvedConfig, err := yaml.Marshal(configMap)
if err != nil {
return errors.New("failed to marshal config after replacing secrets")
}
if err := ValidateOIDCConfig(string(resolvedConfig)); err != nil {
return errors.New("invalid OIDC config")
}
return nil
}
This function parses the raw YAML, replaces any secret references with their actual values, re-serializes the config, and then validates it. Errors are wrapped with generic messages to avoid leaking secret values.
3. Add tests to cover both valid and invalid cases:
func TestValidateOIDCConfigWithSecrets(t *testing.T) {
tests := []struct {
name string
config string
secrets map[string]string
wantErr bool
}{
{
name: "valid secret issuer",
config: `name: Test
issuer: $oidc-secret:issuer
clientID: test
clientSecret: test`,
secrets: map[string]string{
"oidc-secret:issuer": "https://example.com",
},
wantErr: false,
},
{
name: "invalid literal issuer",
config: `name: Test
issuer: ://invalid.example.com
clientID: test
clientSecret: test`,
wantErr: true,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
err := validateOIDCConfigWithSecrets(tc.config, tc.secrets)
if tc.wantErr {
assert.Error(t, err)
} else {
assert.NoError(t, err)
}
})
}
}
This test ensures that a valid issuer value from a secret passes validation, and an invalid literal value fails validation.
From the review thread
No notable review discussion on this PR.
Takeaways
- Always validate configuration after resolving any indirections or references (such as secrets), not before.
- When substituting secrets or sensitive data for validation, avoid leaking those values in error messages or logs.
- Use table-driven tests to cover both success and failure cases, especially when handling configuration with dynamic or secret-backed values.