zarf icon indicating copy to clipboard operation
zarf copied to clipboard

Zarf does not support deploying Flux HelmReleases with OCI Helm chart sources

Open bburky opened this issue 1 year ago • 16 comments

Is your feature request related to a problem? Please describe.

Zarf does not support deploying Flux HelmReleases with OCI Helm chart sources

Related issue

Describe the solution you'd like

  • Given
    • A zarf.yaml with manifests: including a Flux HelmRepository referencing an oci:// chart and a HelmRelease referencing it.
    • And including the OCI helm chart in images: in zarf.yaml.
  • When creating the package
  • Then
    • The package cannot be created because images: does not support arbitrary (non-container) OCI images
      ERROR:  Failed to create package: unable to pull images after 3 attempts: error when trying to save the img                                                                   
              (ghcr.io/stefanprodan/charts/podinfo:6.4.1): mismatched fs layers (1) and diff ids (0)
      
    • Additionally, Zarf's mutating admission webhooks do not support mutating HelmRepository and HelmRelease resources to reference the Zarf registry

Example Flux manifests referencing an OCI Helm chart: https://fluxcd.io/flux/cheatsheets/oci-artifacts/#helm-oci

A potential issue: Zarf uses an insecure registry and Flux source-controller may not support insecure OCI registries with Helm: https://github.com/fluxcd/source-controller/issues/807

  • A little unclear on what's affected and if this issue is still current
  • OCIRepository (consumed by kustomize-controller, not helm-controller) does support .spec.insecure
  • Need to check the current status of HelmRepository and HelmReleases with an insecure localhost registry (sometimes localhost is special cased and allowed)

Describe alternatives you've considered

Zarf's built in Helm support does allow OCI, see demo-helm-oci-chart in this example: https://docs.zarf.dev/examples/helm-charts/

However, this does not support using Flux.

Additional context

The scenario prompting this is a vendor only provides OCI packaged Helm charts. We do not need to modify the upstream vendor charts, and would like to copy them unmodified, and deploy them using Flux.

Additionally, Big Bang now distributes Helm charts in OCI format. If Zarf supported this, the dependency on a Gitea git repository could be dropped in many cases.

Support for generic OCI artifacts may be generally useful for other non-Flux use cases too.

bburky avatar Aug 17 '23 19:08 bburky

+1 this, I would also love to have this exact feature for our Flux Releases

joelcomp1 avatar Aug 31 '23 17:08 joelcomp1

Just starting to investigate this and wanted to circle back to update the issue:

In terms of the admission controller (Zarf Agent), this development looks like it would touch (for Flux OCIArtifacts): HelmRepository

apiVersion: source.toolkit.fluxcd.io/v1beta2
kind: HelmRepository
metadata:
  name: podinfo
  namespace: flux-system
spec:
  interval: 10m
  type: oci
  url: oci://ghcr.io/stefanprodan/charts # <- HERE

OCIRepository

apiVersion: source.toolkit.fluxcd.io/v1beta2
kind: OCIRepository
metadata:
name: podinfo
namespace: flux-system
spec:
interval: 10m
url: oci://ghcr.io/stefanprodan/manifests/podinfo # <- Here 
ref:
  tag: latest

In term's of Zarf's transform library, we should already be capable of transforming the URL as long as we emit the oci:// protocol internally. Example here

The biggest complexity that I am aware based on the issue of seems like it would be around the insecure registry which needs more investigation. I think worst case we would look at securing the Zarf internal registry possibly

cmwylie19 avatar Sep 03 '23 19:09 cmwylie19

There are at least 3 tasks:

  1. Admission webhook update to rewrite HelmRepository and OCIRepository with Zarf registry
  2. Add support for packaging arbitrary OCI artifacts, one option is just telling people to add them to images:. However there is an issue that (crane?) throws the mismatched fs layers (1) and diff ids (0) error mentioned above.
  3. Test if HelmRepository supports an insecure localhost registry.

bburky avatar Sep 05 '23 15:09 bburky

If at all helpful, for #3 listed above, I had to do this internally for a corporate network with Flux HelmRepository and the only work around I could easily get working was to patch the deployment with the CA that I had into the container like mentioned here: https://github.com/fluxcd/flux2/issues/3417#issuecomment-1470961381 i think if its completely insecure (no HTTPS) then I am not sure its at all possible based on what I have tried to do with Flux before and insecure OCI.

joelcomp1 avatar Sep 05 '23 16:09 joelcomp1

There are at least 3 tasks:

  1. Admission webhook update to rewrite HelmRepository and OCIRepository with Zarf registry
  2. Add support for packaging arbitrary OCI artifacts, one option is just telling people to add them to images:. However there is an issue that (crane?) throws the mismatched fs layers (1) and diff ids (0) error mentioned above.
  3. Test if HelmRepository supports an insecure localhost registry.

FWIW, for #(2), i think we have to assume they are bringing their images ready to go. So they basically make a scratch container with their manifests. However, there are other blockers in terms of our internal registry before getting to point 3. I'll leave a comment about it for the future when this can be fixed.

Actually, going to confirm with the team first. Thanks for the updates

cmwylie19 avatar Sep 06 '23 15:09 cmwylie19

So they basically make a scratch container with their manifests

Not quite. These are OCI artifacts (not quite, that specific term can mean something else), not actual "images". These aren't "Docker images" at all (the layers are not application/vnd.oci.image.layer.v1.tar+gzip tarball layers with a application/vnd.oci.image.index.v1+json config). This is packaging non-Docker images as into "OCI images" something still compatible enough with registries to work, but storing different types of data (Helm charts in this case). You'd use something like helm push to create these OCI images.

@Noxsios would be familiar and probably help explain. It's also how Zarf's own OCI support works (Zarf uses ORAS, Helm does something slightly different, but it's the same concept).

And yes you can assume these images already exist, they're created outsize Zarf. Zarf does need to package and transfer them all the way to the Zarf registry somehow though.

In theory these generic OCI images can be copied around the same way as Docker images, which is why I suggested listing them in images:, but it seems something throws an error while downloading them.

bburky avatar Sep 06 '23 22:09 bburky

I'm not able to pull from the internal-docker-registry for some reason, may be oci:// url. Needs more investigation

┌─[cmwylie19@Cases-MacBook-Pro] - [~/hello-zarf] - [2023-09-06 02:45:59]
└─[0] <git:(main 0538a87✱) > k explain ocirepo.spec --recursive
GROUP:      source.toolkit.fluxcd.io
KIND:       OCIRepository
VERSION:    v1beta2

FIELD: spec <Object>

DESCRIPTION:
    OCIRepositorySpec defines the desired state of OCIRepository
    
FIELDS:
  certSecretRef <Object>
    name        <string> -required-
  ignore        <string>
  interval      <string> -required-
  layerSelector <Object>
    mediaType   <string>
  provider      <string>
  ref   <Object>
    digest      <string>
    semver      <string>
    tag <string>
  secretRef     <Object>
    name        <string> -required-
  serviceAccountName    <string>
  suspend       <boolean>
  timeout       <string>
  url   <string> -required-

┌─[cmwylie19@Cases-MacBook-Pro] - [~/zarf] - [2023-09-06 02:46:59]
└─[0] <git:(1974 46635184✱✈) > k get ocirepo hello-zarf -n flux-system -oyaml
apiVersion: source.toolkit.fluxcd.io/v1beta2
kind: OCIRepository
metadata:
  annotations:
    meta.helm.sh/release-name: zarf-1ac628beb6600ac961cf019ef1e52045b7fcbb0a
    meta.helm.sh/release-namespace: flux-system
  creationTimestamp: "2023-09-07T11:56:10Z"
  finalizers:
  - finalizers.fluxcd.io
  generation: 1
  labels:
    app.kubernetes.io/managed-by: Helm
  name: hello-zarf
  namespace: flux-system
  resourceVersion: "22934"
  uid: ef430da6-2ba5-4854-938b-067eef2c520b
spec:
  interval: 10m
  provider: generic
  ref:
    tag: latest-zarf-1662488912
  secretRef:
    name: private-registry
  timeout: 60s
  url: oci://127.0.0.1:31999/cmwylie19/oci-manifests-hello-zarf
status:
  conditions:
  - lastTransitionTime: "2023-09-07T11:56:11Z"
    message: no artifact for resource in storage
    observedGeneration: 1
    reason: NoArtifact
    status: "True"
    type: Reconciling
  - lastTransitionTime: "2023-09-07T11:56:11Z"
    message: 'failed to pull artifact from ''oci://127.0.0.1:31999/cmwylie19/oci-manifests-hello-zarf'':
      Get "https://127.0.0.1:31999/v2/": dial tcp 127.0.0.1:31999: connect: connection
      refused; Get "http://127.0.0.1:31999/v2/": dial tcp 127.0.0.1:31999: connect:
      connection refused'
    observedGeneration: 1
    reason: OCIArtifactPullFailed
    status: "False"
    type: Ready
  - lastTransitionTime: "2023-09-07T11:56:11Z"
    message: 'failed to pull artifact from ''oci://127.0.0.1:31999/cmwylie19/oci-manifests-hello-zarf'':
      Get "https://127.0.0.1:31999/v2/": dial tcp 127.0.0.1:31999: connect: connection
      refused; Get "http://127.0.0.1:31999/v2/": dial tcp 127.0.0.1:31999: connect:
      connection refused'
    observedGeneration: 1
    reason: OCIArtifactPullFailed
    status: "True"
    type: FetchFailed
  observedGeneration: -1

┌─[cmwylie19@Cases-MacBook-Pro] - [~/zarf] - [2023-09-06 02:48:37]
└─[0] <git:(1974 46635184✱✈) > oras repo tags localhost:5000/cmwylie19/oci-manifests-hello-zarf
latest
latest-zarf-1662488912

cmwylie19 avatar Sep 07 '23 13:09 cmwylie19

@bburky thank you for the clarity!!

cmwylie19 avatar Sep 07 '23 17:09 cmwylie19

Just to clarify on flux support - OCIRepository cannot be used as a source for a HelmRelease. So I believe to fully support OCI sourced Flux HelmReleases with Zarf we'll need to solve the insecure registry issue either by Flux adding support or by securing the Zarf registry.

mjnagel avatar Sep 20 '23 12:09 mjnagel

Unassigned as this will be likely 2 PRs, one for OCIRepo and another that needs more discussion between Zarf team. OCIRepo should be ready soon but is pending one PR.

The current PR that I am working on is addressed by this issue (Which is specific for OCIRepo)

cmwylie19 avatar Sep 20 '23 13:09 cmwylie19

The challenge with tls on the registry is their is no safe k8s-native way to tell the CRIs to use a given CA like you can with admission controllers. The most generic k8-way is to use a privileged daemonset to modify your CRI config (which is as awful as it sounds). Many platforms will modify the CRI out-of-band to accommodate this during cluster init or node image building.

jeff-mccoy avatar Sep 20 '23 13:09 jeff-mccoy

re-assigning myself after chatting with @Racer159 , we will support HelmRepo as long as the type is OCI

---
apiVersion: source.toolkit.fluxcd.io/v1beta2
kind: HelmRepository
metadata:
  name: podinfo
  namespace: default
spec:
  type: "oci"
  interval: 5m0s
  url: oci://ghcr.io/stefanprodan/charts

cmwylie19 avatar Sep 21 '23 14:09 cmwylie19

  • In terms of the HelmRepo, I ultimately ended up getting blocked by Flux not supporting insecure registry as @bburky eluded to in the initial message. However, the logic mutate the admission controller is there in this draft PR, so once Flux adds support, we can revisit this and merge it into Zarf.

  • The logic for OCIRepo is done and will close this issue and will live on another branch.

cmwylie19 avatar Sep 27 '23 12:09 cmwylie19

Flux added a .spec.insecure to type: oci HelmRepository resources, the flux issue linked above was closed. 🎉

In addition to mutating the source of an OCI HelmRepository/OCIRepository, zarf could add the insecure flag automatically if a localhost registry is used.

bburky avatar Dec 14 '23 20:12 bburky

For now, git server is required in local cluster if deploy using flux. Just require the local registry if oci is supported.

koolay avatar Jan 01 '24 09:01 koolay

I see that this has been unblocked now, what is the latest update on this feature?

shanewhite97 avatar Mar 25 '24 15:03 shanewhite97