diff --git a/api/v1/condition_types.go b/api/v1/condition_types.go index 3bd3b70c..9641db99 100644 --- a/api/v1/condition_types.go +++ b/api/v1/condition_types.go @@ -111,4 +111,8 @@ const ( // InvalidSTSConfigurationReason signals that the STS configurtion is invalid. InvalidSTSConfigurationReason string = "InvalidSTSConfiguration" + + // InvalidProviderConfigurationReason signals that the provider + // configuration is invalid. + InvalidProviderConfigurationReason string = "InvalidProviderConfiguration" ) diff --git a/internal/controller/gitrepository_controller.go b/internal/controller/gitrepository_controller.go index 618a5e7b..b741d876 100644 --- a/internal/controller/gitrepository_controller.go +++ b/internal/controller/gitrepository_controller.go @@ -667,7 +667,7 @@ func (r *GitRepositoryReconciler) getAuthOpts(ctx context.Context, obj *sourcev1 if obj.Spec.SecretRef == nil { e := serror.NewStalling( fmt.Errorf("secretRef with github app data must be specified when provider is set to github"), - sourcev1.AuthenticationFailedReason, + sourcev1.InvalidProviderConfigurationReason, ) conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e) return nil, e @@ -684,7 +684,7 @@ func (r *GitRepositoryReconciler) getAuthOpts(ctx context.Context, obj *sourcev1 if appID := authData[github.AppIDKey]; len(appID) != 0 { e := serror.NewStalling( fmt.Errorf("secretRef '%s/%s' has github app data but provider is not set to github", obj.GetNamespace(), obj.Spec.SecretRef.Name), - sourcev1.AuthenticationFailedReason, + sourcev1.InvalidProviderConfigurationReason, ) conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e) return nil, e diff --git a/internal/controller/gitrepository_controller_test.go b/internal/controller/gitrepository_controller_test.go index 616a9b34..d278e41d 100644 --- a/internal/controller/gitrepository_controller_test.go +++ b/internal/controller/gitrepository_controller_test.go @@ -572,6 +572,50 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) { *conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new upstream revision 'master@sha1:'"), }, }, + { + // This test is only for verifying the failure state when using + // provider auth. Protocol http is used for simplicity. + name: "github provider without secret ref makes FetchFailed=True", + protocol: "http", + beforeFunc: func(obj *sourcev1.GitRepository) { + obj.Spec.Provider = sourcev1.GitProviderGitHub + conditions.MarkReconciling(obj, meta.ProgressingReason, "foo") + conditions.MarkUnknown(obj, meta.ReadyCondition, meta.ProgressingReason, "foo") + }, + want: sreconcile.ResultEmpty, + wantErr: true, + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.InvalidProviderConfigurationReason, "secretRef with github app data must be specified when provider is set to github"), + *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"), + *conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "foo"), + }, + }, + { + // This test is only for verifying the failure state when using + // provider auth. Protocol http is used for simplicity. + name: "empty provider with github app data in secret makes FetchFailed=True", + protocol: "http", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "github-app-secret", + }, + Data: map[string][]byte{ + github.AppIDKey: []byte("1111"), + }, + }, + beforeFunc: func(obj *sourcev1.GitRepository) { + obj.Spec.SecretRef = &meta.LocalObjectReference{Name: "github-app-secret"} + conditions.MarkReconciling(obj, meta.ProgressingReason, "foo") + conditions.MarkUnknown(obj, meta.ReadyCondition, meta.ProgressingReason, "foo") + }, + want: sreconcile.ResultEmpty, + wantErr: true, + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.InvalidProviderConfigurationReason, "secretRef '/github-app-secret' has github app data but provider is not set to github"), + *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"), + *conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "foo"), + }, + }, } for _, tt := range tests { @@ -710,17 +754,6 @@ func TestGitRepositoryReconciler_getAuthOpts_provider(t *testing.T) { wantProviderOptsName: sourcev1.GitProviderGitHub, wantErr: errors.New("secretRef with github app data must be specified when provider is set to github"), }, - { - name: "github provider with secret ref that does not exist", - url: "https://github.com/org/repo.git", - beforeFunc: func(obj *sourcev1.GitRepository) { - obj.Spec.Provider = sourcev1.GitProviderGitHub - obj.Spec.SecretRef = &meta.LocalObjectReference{ - Name: "githubAppSecret", - } - }, - wantErr: errors.New("failed to get secret '/githubAppSecret': secrets \"githubAppSecret\" not found"), - }, { name: "github provider with github app data in secret", url: "https://example.com/org/repo", @@ -768,6 +801,16 @@ func TestGitRepositoryReconciler_getAuthOpts_provider(t *testing.T) { obj.Spec.Provider = sourcev1.GitProviderGeneric }, }, + { + name: "secret ref defined for non existing secret", + url: "https://github.com/org/repo.git", + beforeFunc: func(obj *sourcev1.GitRepository) { + obj.Spec.SecretRef = &meta.LocalObjectReference{ + Name: "authSecret", + } + }, + wantErr: errors.New("failed to get secret '/authSecret': secrets \"authSecret\" not found"), + }, { url: "https://example.com/org/repo", name: "no provider",