Improve kapp-ctrl alignment

[antgamdia]

Description of the change

After digging into the kapp-ctrl code, I noticed we were doing a few things a little bit different from their approach (missing annotations, incomplete naming patterns, etc). This PR is to change a few things I've encountered when dealing with PackageInstalls and their associated Secrets.

Benefits

Our generated k8s resources will be similar to the ones created with kapp-ctrl.

Possible drawbacks

N/A

Applicable issues

• rel Remaining work for running Kubeapps with Carvel #3849

Additional information

I still have to check if the Secret object we create gets automatically deleted by kapp-ctrl under the hood (now we are creating it with the proper annotation); if so, we can simplify the logic for deleting a package.
No, it doesn't

[castelblanque]

Just for my knowledge, why do we suddenly get all these added? 😄

[antgamdia]

They are the deps introduced by the Carvel stack, it seems. Also happened in this other PR https://github.com/kubeapps/kubeapps/pull/4068/files

The go cli asked me to run: go mod tidy -go=1.16 && go mod tidy -go=1.17 because of some conflicts I guess.

[absoludity]

I'm a bit confused why we're adding the annotations, assuming that the kapp controller CLI adds them to the secret it creates, annotating them as such, but do we want to annotate the secrets we create as if they're created by the kapp controller cli? Or maybe the annotation is more general than that? What happens, Kubeapps aside, if a user creates a secret, then passes that secret to the CLI during a package creation? (don't remember).

[absoludity]

Let me know if I missed something, but it looks like none of these test changes are actually testing the change (of adding the annotations?)

[antgamdia]

Yep, right, they aren't actually testing it, but definitively we should test it. I'll do it.

do we want to annotate the secrets we create as if they're created by the kapp controller cli? Or maybe the annotation is more general than that?

Currently, the annotation being added is packaging.carvel.dev/package-secret=myPkgName-myNS-values (previously it was tkg.tanzu.vmware.com/tanzu-package), so I just thought it would be a good idea to also use it. I mean, there is nothing strictly related to kapp-controller, and -not sure- not adding it might have some unexpected side effects for those users also using kapp CLI for managing the packages they installed via Kubeapps.

Besides, another reason for adding them is that I initially thought it could be beneficial for getting some auto-deletion of some resources (like secrets) when uninstalling an app. However, it doesn't happen.

I'd rather keep the annotations - plus updating the tests - (as they are harmless) just to give the same CLI experience if a user decides to use kapp for managing kubeapps-installed carvel packages eventually. Not a strong opinion, though; happy to remove them if you think otherwise.

[absoludity]

That's fine with me. I just wasn't sure:

What happens, Kubeapps aside, if a user creates a secret, then passes that secret to the [kapp] CLI during a package creation? (don't remember).

From what you've said (about it being the same CLI experience), it sounds like kapp will annotate this user-created secret (which surprised me - I'd expect kapp to add such an annotation to a secret that it creates on behalf of the user, not one supplied by the user).

+1

[antgamdia]

Well... I've been looking into this after merging. I made a mistake: I assumed the annotations I mentioned were added by kapp-controller's code, but it's not.
kapp-ctrl people are developing their own CLI (that is, not the kapp CLI, but the kctrl CLI instead)

The annotations I assumed were added by kapp-ctrl were, actually, added by this new (and unreleased) kctrl CLI.
This is so confusing, I mean, in that the annotations added in each case are totally different.

PackageInstall

Using kubectl apply

  annotations:
    # <none>
  labels:
    # <none>

Using kapp deploy

  annotations:
    kapp.k14s.io/identity: v1;default/packaging.carvel.dev/PackageInstall/pkg-demo;packaging.carvel.dev/v1alpha1
    kapp.k14s.io/original: '{"apiVersion":"packaging.carvel.dev/v1alpha1","kind":"PackageInstall","metadata":{"labels":{"kapp.k14s.io/app":"1643381406628771860","kapp.k14s.io/association":"v1.9c2b0de727437c93dd7e5dcc44c83cb1"},"name":"pkg-demo","namespace":"default"},"spec":{"packageRef":{"refName":"simple-app.corp.com","versionSelection":{"constraints":"1.0.0"}},"serviceAccountName":"default-ns-sa","values":[{"secretRef":{"name":"pkg-demo-values"}}]}}'
    kapp.k14s.io/original-diff-md5: c6e94dc94aed3401b5d0f26ed6c0bff3
  labels:
    kapp.k14s.io/app: "1643381406628771860"
    kapp.k14s.io/association: v1.9c2b0de727437c93dd7e5dcc44c83cb1

Using kctrl package install

  annotations:
    packaging.carvel.dev/package-ClusterRole: demo-cli-1-default-cluster-role
    packaging.carvel.dev/package-ClusterRoleBinding: demo-cli-1-default-cluster-rolebinding
    packaging.carvel.dev/package-Secret: demo-cli-1-default-values
    packaging.carvel.dev/package-ServiceAccount: demo-cli-1-default-sa
    tkg.tanzu.vmware.com/tanzu-package-ClusterRole: demo-cli-1-default-cluster-role
    tkg.tanzu.vmware.com/tanzu-package-ClusterRoleBinding: demo-cli-1-default-cluster-rolebinding
    tkg.tanzu.vmware.com/tanzu-package-Secret: demo-cli-1-default-values
    tkg.tanzu.vmware.com/tanzu-package-ServiceAccount: demo-cli-1-default-sa
  labels:
    # <none>

Secret (created by passing the values)

Using kubectl apply

  annotations:
    # <none>
  labels:
    # <none>

Using kapp deploy

  annotations:
    kapp.k14s.io/identity: v1;default/packaging.carvel.dev/PackageInstall/pkg-demo;packaging.carvel.dev/v1alpha1
    kapp.k14s.io/original: '{"apiVersion":"packaging.carvel.dev/v1alpha1","kind":"PackageInstall","metadata":{"labels":{"kapp.k14s.io/app":"1643381406628771860","kapp.k14s.io/association":"v1.9c2b0de727437c93dd7e5dcc44c83cb1"},"name":"pkg-demo","namespace":"default"},"spec":{"packageRef":{"refName":"simple-app.corp.com","versionSelection":{"constraints":"1.0.0"}},"serviceAccountName":"default-ns-sa","values":[{"secretRef":{"name":"pkg-demo-values"}}]}}'
    kapp.k14s.io/original-diff-md5: c6e94dc94aed3401b5d0f26ed6c0bff3
  labels:
    kapp.k14s.io/app: "1643381406628771860"
    kapp.k14s.io/association: v1.9c2b0de727437c93dd7e5dcc44c83cb1

Using kctrl package install

  annotations:
    packaging.carvel.dev/package: demo-cli-1-default
    tkg.tanzu.vmware.com/tanzu-package: demo-cli-1-default
  labels:
    # <none>

So, as always in case of doubt, let me raise the question to the Carvel team to see what they think is the best way to go (for a 3rd party PackageInstall creator like Kubeapps, so to speak).
I'll send a PR reverting the changes in the annotations right now, but will modify it according to what Carvel people say.

[antgamdia]

Carvel team answered: we shouldn't add any annotation.