Tilt: Duplicate YAML Entries when calling `helm()`

[KOGI]

This behavior seems to have started with Tilt v0.17.0. I originally thought it was related to the helm_remote() extension, but have since traced it back to Tilt itself (via helm()).

Problem:

Tilt started on http://localhost:10350/
v0.17.0, built 2020-07-31

Initial Build • (Tiltfile)
Beginning Tiltfile execution
Running: [helm template chart /path/to/helmchart --include-crds]
Successfully loaded Tiltfile (373.0113ms)
Duplicate YAML  #Entity:
certgen:serviceaccount:default:core has been detected across one or more resources.
Only one specification per entity can be applied to the cluster;
to ensure expected behavior, remove the duplicate specifications.

• This is attempting to install a local copy of the raw helmchart directly from the repo.
• No modifications have been made to the helmchart whatsoever.
• This is the _only_ command in the Tiltfile

Reproduce Steps:

1. run
   helm repo add gloo https://storage.googleapis.com/solo-public-helm && helm pull gloo/gloo --untar --destination /path/to/helmcharts
2. create a Tiltfile with the following contents:
   k8s_yaml(helm('/path/to/helmcharts/gloo'))
3. run
   tilt up

Additional Thoughts

It would be super helpful if the above error message were to tell the user where these duplicates were found. This would allow us to much more easily locate a problem in our tiltfile.

[jazzdan]

Hmm, yeah this is working as intended because that chart _does_ define certgen:serviceaccount:default:core twice:

# Source: gloo/templates/6.5-gateway-certgen-job.yaml
apiVersion: v1
kind: ServiceAccount
metadata:
  labels:
    app: gloo
    gloo: rbac
  annotations:
    "helm.sh/hook": pre-install
    "helm.sh/hook-weight": "5" # must be executed before cert-gen job
  name: certgen
  namespace: default

---

# Source: gloo/templates/6.5-gateway-certgen-job.yaml
# Resources created as hooks are not considered part of a release by Helm. This means that they
# will not be cleaned up when running `helm uninstall`. The ways to get around this are documented here:
# https://helm.sh/docs/topics/charts_hooks/#hook-resources-are-not-managed-with-corresponding-releases).
# These will not work for us though: we can't use `hook-delete-policy` since we have multiple hooks that depend
# on each other (RBAC resources needed by the job), and we cannot write a job to clean them up as this would result
# in a catch-22 (this second job would in turn need its own RBAC resources and who would clean up those?).
#
# To be able to clean up these hook resources, which are needed only temporarily during the pre-install phase,
# we redefine them as `post-install` hooks with a `hook-delete-policy`. This way Helm will reapply them and
# immediately delete them after the installation completes. Note that we have to explicitly define a `before-hook-creation`
# policy as well, to avoid failing on existing resources (`before-hook-creation` is the default `hook-delete-policy`
# if none is specified`).
#
# The following resources implement this workaround.
apiVersion: v1
kind: ServiceAccount
metadata:
  labels:
    app: gloo
    gloo: rbac
  annotations:
    "helm.sh/hook": post-install
    "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded
    "solo.io/hook-cleanup": "true" # Used internally to mark "hook cleanup" resources
  name: certgen
  namespace: default

If I'm reading that comment on the second one correctly this may be due to some work that this chart is doing to get around some limitations in Helm's hook functionality.

My first thought if we could use filter_yaml to filter out the duplicate one. Unfortunately the fact that these two resources differ only by their annotations makes this hard.

My next thought was that we could somehow manually remove the duplicate by using filter_yaml to select the duplicates and then removing one of them.

orig = helm('gloo')
tofilter, rest = filter_yaml(helm('gloo'), name='certgen', kind='ServiceAccount')
k8s_yaml(rest)

yaml = decode_yaml_stream(tofilter)
# drop one of them
certgen = yaml[0]
k8s_yaml(encode_yaml(certgen))

Unfortunately then there's another duplicate entry, and I stopped here for the day.

Duplicate YAML Entity: gloo-gateway-validation-webhook-default:validatingwebhookconfiguration:default:admissionregistration.k8s.io has been detected across one or more resources.  Only one specification per entity can be applied to the cluster; to ensure expected behavior, remove the duplicate specifications.

Maybe there should be a way for you to opt in and "ignore duplicates"? Let me think about this:

It would be super helpful if the above error message were to tell the user where these duplicates were found. This would allow us to much more easily locate a problem in our tiltfile.

Agreed. @abdullahzameek do you have any thoughts on how we would surface this?

[KOGI]

Maybe there should be a way for you to opt in and "ignore duplicates"? Let me think about this.

@jazzdan I know you guys all have your own priorities, too, but just to make you guys aware... this 100% breaks my entire workspace. It looks like it's also an issue with v0.16.0 as well. Our team can manually downgrade to v0.15.0 for now, but this is a pretty major problem for our project, so any effort you/the team are able to put toward this will be hugely appreciated!

[abdullahzameek]

It would be super helpful if the above error message were to tell the user where these duplicates were found. This would allow us to much more easily locate a problem in our tiltfile.

Looking through this to see how/if this can be done!

[nicks]

Ya, I'm intrigued by the long-term possibility of having a way for tilt read annotations and using that to enforce ordering, or of having a better understanding of helm hooks, or of having systems that update a resource multiple times at different stages of the deploy lifecycle.

But totally agree that we need a short-term fix for this for the next release!!

[landism]

I think it'd be cheap to write an extension that removes duplicates from a yaml blob, with an optional comparison func arg the caller can pass if they care how conflicts are resolved.

[nicks]

i think i have an idea for some quick fixes here that will at least let us move forward

[nicks]

tilt v0.17.1 should have a k8s_yaml(allow_duplicates=True) https://github.com/tilt-dev/tilt/issues/3656

Going to fix up helm_remote -- it might even make sense for helm_remote to always set allow_duplicates=True