kapp-controller icon indicating copy to clipboard operation
kapp-controller copied to clipboard

`kctrl`: Allow users to use ytt overlay while creating package installs easily

Open 100mik opened this issue 3 years ago • 11 comments

Describe the problem/challenge you have Allow users to supply an overlay file or inline (simiolar to how we handle values file) that can be fed into the ext.packaging.carvel.dev/ytt-paths-from-secret-name annotation.

Describe the solution you'd like A flag like --overlay which can be used multiple times (similar to how kapp accepts multiple -f flag) which can be used to create multiple overlaying annotations.

Acceptance criteria:

  • [x] Creates secrets with ownership annotations using overlays provided by the user
  • [x] These secrets should be added as overlays via annotation
  • [x] They should be garbage collected while uninstalling
  • [ ] Tests

Open questions

  • Should we allow multiple overlays? Does this help users organise their overlays better?
  • Should we create one secret with multiple file keys?
  • How do we handle updates? Do we merge if only one file is provided? Or do we clobber and overwrite the list of overlays each time? (not merging would be consistent with how we handle values secrets today)

Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this addressed as soon as possible" 👎 "There are other more important things to focus on right now"

We are also happy to receive and review Pull Requests if you want to help working on this issue.

100mik avatar May 10 '22 07:05 100mik

Branched out from #412 Relevant documentation on the cluster side of things : https://carvel.dev/kapp-controller/docs/v0.36.1/package-install-extensions/#adding-paths-to-ytt-overlays

100mik avatar May 10 '22 07:05 100mik

First off, would love to make sure we have a --dry-run or similar option, since most of my exposure to using this annotation is then committing the results to git instead of applying then and there.

We use this annotation a lot, and it comes up for several reasons:

  1. This is (as you mentioned) the only way to apply an overlay to a PackageInstall
  2. We store everything as an App CR that creates PackageInstalls, and we need to store SOPS encrypted files as wrapped secrets, we then apply that secret into the cluster and link it to the PackageInstall with this annotation.
  3. While the primary motivation are overlays, you can also use this to "bundle" other manifests into a Package -- things like roles, namespaces, or other things that the package itself should handle but doesn't.

Regarding:

Should we create one secret with multiple file keys? We don't usually do this as it's very confusing for newer users, I realize it's possible (and we've done it on occasion) but it makes things a little harder to understand so I'd recommend against it but only weakly and can be convinced otherwise.

voor avatar May 10 '22 09:05 voor

Also, I really think given the utilization of these use cases we should really move this out of the annotations and into an actual field.

voor avatar May 10 '22 10:05 voor

That does make a lot of sense, thanks!

Re:

Also, I really think given the utilization of these use cases we should really move this out of the annotations and into an actual field.

From a kctrl perspective I believe we will have to use the annotations till we are sure everyone has moved on to a newer version, if we decide to make that change in kapp-controller I imagine kapp-controller might support both for a while as well.

100mik avatar May 10 '22 11:05 100mik

As discussed over slack, I use this to provide an additional capability that one of the packages that I use does not provide:

apiVersion: packaging.carvel.dev/v1alpha1
kind: PackageInstall
metadata:
  name: contour
  namespace: tce-packages
  annotations:
    ext.packaging.carvel.dev/ytt-paths-from-secret-name.0: contour-tce-packages-ingressclass-overlay
spec:
  paused: #@ data.values.packageinstall.paused
  canceled: #@ data.values.packageinstall.canceled
  noopDelete: #@ data.values.packageinstall.noopDelete
  syncPeriod: #@ data.values.packageinstall.syncPeriod
  serviceAccountName: contour-tce-packages-sa
  packageRef:
    refName: contour.community.tanzu.vmware.com
    versionSelection:
      constraints: 1.20.1
      prereleases: {}
  values:
    - secretRef:
        name: contour-tce-packages-values
---
apiVersion: v1
kind: Secret
metadata:
  name: contour-tce-packages-ingressclass-overlay
  namespace: tce-packages
stringData:
  contour-ingressclass-overlay.yaml: |
    ---
    apiVersion: networking.k8s.io/v1
    kind: IngressClass
    metadata:
      name: contour-default
      annotations:
        "ingressclass.kubernetes.io/is-default-class": "true"
    spec:
      controller: projectcontour.io/contour

The main issue I currently have is that there's no way to do this via the CLI so I need to install packages manually (or patch them) after created via the CLI.

Another example:

apiVersion: packaging.carvel.dev/v1alpha1
kind: PackageInstall
metadata:
  name: kubeapps
  namespace: tce-packages
  annotations:
    ext.packaging.carvel.dev/ytt-paths-from-secret-name.0: kubeapps-tce-packages-overlay
    ext.packaging.carvel.dev/helm-template-name: kubeapps
    ext.packaging.carvel.dev/helm-template-namespace: kubeapps
spec:
  paused: #@ data.values.packageinstall.paused
  canceled: #@ data.values.packageinstall.canceled
  noopDelete: #@ data.values.packageinstall.noopDelete
  syncPeriod: #@ data.values.packageinstall.syncPeriod
  serviceAccountName: kubeapps-tce-packages-sa
  packageRef:
    refName: kubeapps.community.tanzu.vmware.com
    versionSelection:
      constraints: 8.0.13
      prereleases: {}
  values:
    - secretRef:
        name: kubeapps-tce-packages-values
---
apiVersion: v1
kind: Secret
metadata:
  name: kubeapps-tce-packages-overlay
  namespace: tce-packages
stringData:
  contour-ingressclass-overlay.yaml: |
    #@ load("@ytt:overlay", "overlay")
    #@ load("@ytt:data", "data")
    #! Add kubeapps namespace
    ---
    apiVersion: v1
    kind: Namespace
    metadata:
      name: "kubeapps"
    #! Add HTTPProxy. We need to make sure that contour Ingress Controller is deployed, as this relies on Contour HTTPProxy
    ---
    kind: HTTPProxy
    apiVersion: projectcontour.io/v1
    metadata:
      name: kubeapps-grpc
      namespace: kubeapps
    spec:
      virtualhost:
        fqdn: #@ data.values.ingress.hostname
      routes:
        - conditions:
          - prefix: /apis/
          pathRewritePolicy:
            replacePrefix:
            - replacement: /
          services:
          - name: kubeapps-internal-kubeappsapis
            port: 8080
            protocol: h2c
        - services:
          - name: kubeapps
            port: 80
    #@overlay/match by=overlay.subset({"kind":"Deployment", "metadata":{"name": "kubeapps-internal-apprepository-controller"}})
    ---
    spec:
      template:
        spec: 
          containers:
            #@overlay/match by="name"   
            - name: controller
              args:
              #@overlay/match by=overlay.subset("--user-agent-comment=kubeapps/2.4.4")
              #@overlay/replace via=lambda left, right: left.replace("kubeapps/2.4.4", right)
              - tanzu-community-extension-for-docker-desktop/v0.0.1
    #! AUTH Token
    ---
    apiVersion: v1
    kind: ServiceAccount
    metadata:
      name: token-user
      namespace: kubeapps
    ---
    apiVersion: rbac.authorization.k8s.io/v1
    kind: ClusterRoleBinding
    metadata:
      name: token-user
    roleRef:
      apiGroup: rbac.authorization.k8s.io
      kind: ClusterRole
      name: cluster-admin
    subjects:
    - kind: ServiceAccount
      name: token-user
      namespace: kubeapps

jorgemoralespou avatar May 10 '22 11:05 jorgemoralespou

This is very much needed at this point, as many of the packages used (specially in TCE) don't provide enough configuration for some users, resorting to other solutions like adding "skip" on update-strategy for some configmaps, so that configuration can be modified manually.

jorgemoralespou avatar May 24 '22 14:05 jorgemoralespou

In the draft PR for this, The use can supply multiple --ytt-overlay-file flags pointing to paths to overlay files or directories with overlay files. --ytt-overlays=false can be used to drop values file similar to --values and --values-file.

If multiple overlays are supplied they are ordered in the order which they are supplied. In the case of a folder with multiple files, the files will be ordered alphabetically.

100mik avatar Jun 21 '22 11:06 100mik

@100mik can you elaborate on the --ytt-overlays use case?

jorgemoralespou avatar Jun 21 '22 13:06 jorgemoralespou

or directories with overlay files.

This functionality in particular would be great even for gitops use cases, having the ability to dump my files into a directory and then kctrl blah blah -f my_directory and dump out a well formatted secret that already injects all of the files would be super handy. I don't think even kubectl has that kind of smartness when --dry-run creating secrets.

voor avatar Jun 21 '22 13:06 voor

@jorgemoralespou If a user wants to stop using ytt overlays that were previously supplied to a package installation, they can do something like kctrl package installed update -i <pkgi-name> --ytt-overlays=false.

This flag is true by default, which means that kctrl will keep existing overlays supplied to it. When set to false, it will delete the created secret and remove it's reference from the package installation.

100mik avatar Jun 21 '22 18:06 100mik

I see. Doesn't feel intuitive, TBH. I would name the flag something more obvious, like --remove-overlays and have it default to false. It's easier to understand.

jorgemoralespou avatar Jun 21 '22 19:06 jorgemoralespou

We did go with --ytt-overlays as it was consistent with how we handle values

100mik avatar Sep 20 '22 20:09 100mik