Skip to content
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.

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

Confusing HelmRepository OCI status #1249

Closed
3 tasks done
darkowlzz opened this issue Oct 3, 2023 · 0 comments · Fixed by #1243
Closed
3 tasks done

Confusing HelmRepository OCI status #1249

darkowlzz opened this issue Oct 3, 2023 · 0 comments · Fixed by #1243
Labels
area/helm Helm related issues and pull requests area/oci OCI related issues and pull requests enhancement New feature or request

Comments

@darkowlzz
Copy link
Contributor

darkowlzz commented Oct 3, 2023

HelmRepository OCI, implemented as per RFC-0002, has a dedicated reconciler, HelmRepositoryOCIReconciler, which reconciles the HelmRepository objects with .spec.type value oci and patches the result of reconciliation in the object status. The reconciliation involves validation of the repository URL and performing a login to the OCI registry.

Since the release of HelmRepository OCI support, there have been multiple reports about HelmRepository showing everything to be good and ready but HelmChart failing to pull the OCI chart and confusing the users. For example, a HelmRepository

Spec:
  Interval:  2m0s
  Secret Ref:
    Name:   registry-token
  Timeout:  60s
  Type:     oci
  URL:      oci://my_domain.azurecr.io/helm/mongodb
Status:
  Conditions:
    Last Transition Time:     2022-07-20T20:11:58Z
    Message:                  Helm repository is ready
    Observed Generation:      2
    Reason:                   Succeeded
    Status:                   True
    Type:                     Ready
  Last Handled Reconcile At:  2022-07-21T18:04:30.364482+02:00
  Observed Generation:        2

Corresponding HelmChart fails with the following error:

{"level":"error","ts":"2022-07-21T18:34:48.090Z","logger":"controller.helmchart","msg":"Reconciler error","reconciler group":"source.toolkit.fluxcd.io","reconciler kind":"HelmChart","name":"mongodb-mongodb","namespace":"mongodb","error":"chart pull error: chart pull error: failed to get chart version for remote reference: GET \"https://my_domain.azurecr.io/v2/helm/mongodb/mongodb/tags/list\": unexpected status code 401: unauthorized: authentication required, visit https://aka.ms/acr/authorization for more information."}

Because the initial HelmRepository implementation in Flux was based on the non-OCI helm design where the HelmRepository downloaded helm chart index and the index is used by HelmCharts, HelmRepository OCI using the same HelmRepository API may lead the users to believe that any authentication failure would happen in HelmRepository and be visible in HelmRepository object itself. But since HelmRepository OCI doesn't provide any artifact that the corresponding HelmCharts need, that's not the case anymore. For OCI charts, HelmRepository is just a container to store information about how OCI registry from where the charts can be downloaded. The HelmRepository OCI object itself doesn't contain any information about the individual charts. The underlying HelmRepositoryOCIReconciler can't perform accurate authentication and guarantee that that HelmCharts will be able to authenticate too. HelmRepository OCI performs authentication against the registry host. It's possible that the credentials may not have pull access to particular charts in the registry.

As mentioned before, HelmRepositoryOCIReconciler only performs URL validation and registry host authentication, it doesn't provide any artifact. The associated HelmCharts perform authentication when pulling the OCI charts. This leaves HelmRepository OCI to provide very less value compared to the other source reconcilers. HelmChart reconciler does all the necessary operations for OCI charts. Since HelmCharts can work without running a dedicated reconciler just for validation and registry host authentication for HelmRepository, removing the HelmRepositoryOCIReconciler will not make any difference. The HelmRepository OCI objects can become static object without any reconciler reporting any status. This would help reduce the confusion.

NOTE: This change is inspired by the fluxcd/notification-controller#540 which introduces static Alerts and Providers API in notification-controller. Prerequisites for static objects have been discussed in the notification-controller change and addressed in the recent versions of Flux.

Following are some issues and slack threads where this confusion was observed:

TODOs for addressing the issue:

@darkowlzz darkowlzz added enhancement New feature or request area/helm Helm related issues and pull requests area/oci OCI related issues and pull requests labels Oct 3, 2023
@darkowlzz darkowlzz changed the title Misleading HelmRepository OCI status Confusing HelmRepository OCI status Oct 3, 2023
@darkowlzz darkowlzz reopened this Nov 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/helm Helm related issues and pull requests area/oci OCI related issues and pull requests enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant