[OCI] Static credentials should take precedence over the OIDC provider

[souleb]

fixes #874

If spec.SecretRef or spec.ServiceAccountName is provided, the controller will use the static credentials from the referenced secret.

Otherwise, if a non-generic provider is present in the definition, the controller will try to resolve credentials from the provider.

Signed-off-by: Soule BA soule@weave.works

[stefanprodan]

LGTM

Thanks @souleb

[souleb]

This has been tested on eks:

• validate that a helmrepo/release with both spec.secretRef and provider, uses the spec.secretRef
• validate that a helmrepo/release with only provider set to aws login successfully
• validate that a helmrepo/release with provider not set, reconcile successfully for public registries.
• validate that a ocirepository with both spec.secretRef and provider, uses the spec.secretRef
• validate that a ocirepository with only provider set to aws login successfully
• validate that a ocirepository with provider not set, reconcile successfully for public registries.

[stefanprodan]

Thanks @souleb for all the testing 🥇

[souleb]

please don't merge this. I have to to add the tests for the HelmChartReconciler.

[souleb]

@darkowlzz this is ready now.

Note that the helmChart reconciler return *conditions.TrueCondition(sourcev1.FetchFailedCondition, "Unknown", "unknown build error: failed to login to OCI registry"), when we have an authentication error. This is due to observeChartBuild(obj *sourcev1.HelmChart, build *chart.Build, err error) returning an Reason: chart.ErrUnknown when the error is not a *chart.BuildError. We will have to change that to give more context to the user.

[darkowlzz]

Note that the helmChart reconciler return *conditions.TrueCondition(sourcev1.FetchFailedCondition, "Unknown", "unknown build error: failed to login to OCI registry"), when we have an authentication error. This is due to observeChartBuild(obj *sourcev1.HelmChart, build *chart.Build, err error) returning an Reason: chart.ErrUnknown when the error is not a *chart.BuildError. We will have to change that to give more context to the user.

For the tests and the scenarios that result in unknown build error and Unknown reason with auth failure, it happens when HelmChart build is attempted when the associated HelmRepository isn't ready because of the auth failure. Around

source-controller/controllers/helmchart_controller.go

Line 388 in e22a664

// Assert source has an artifact

we have a check for non-OCI charts to ensures that the HelmRepo has artifact before proceeding further. We can add a similar check for OCI where we make sure that the HelmRepo is ready before proceeding to build it. That way, we can avoid such Unknown errors and show better status conditions.
I think it's not a big concern to be addressed in this PR. We can do it separately later.

[souleb]

@darkowlzz I put your comment in an issue: #894.

[darkowlzz]

LGTM!
I suspect that the CI failure may be due to the existence of AWS secrets in the env vars that were added in github actions recently. The contextual login tries to use the credentials to actually log into a non-existing repository and fails.