meta: Treat internal k8s annotations as invalid

[radeksimko]

We always ignored internal annotations and we never allowed users to update them either as an effect of another bug. This is making it much more explicit and upfront and hopefully less confusing.

Closes #37

Why we don't support internal annotations

Annotations are more often (compared to labels) used by the scheduler or machine generally rather than a human. We cannot tell the difference between annotation specified by a user and machine-managed one from the API. This may be "by design" and we're not complaining about the K8S annotations design here by any means. It's a design preventing Terraform from detecting drifts between reality and config nonetheless.

This conflict of ownership presents a problem for any field of any resource and provider. Internal (kubernetes.io/*) annotations are just well known to be used by kubelet out of the box.

If you have any tools/scripts outside of Terraform which add, remove or modify annotations for objects managed by Terraform, you may use ignore_changes, i.e.

  lifecycle {
    ignore_changes = ["metadata.0.annotations"]
  }

This is a way to tell Terraform that you manage annotations separately and want Terraform to ignore any changes your tools/scripts make to annotations.

Vast majority of custom user-specifyable annotations are also related to alpha or beta features which we currently don't intend to support as mentioned in Readme with further explanation in #1 (comment)

Before

$ terraform apply
...
$ terraform plan

  ~ kubernetes_pod.echo
      metadata.0.annotations.%:                      "0" => "1"
      metadata.0.annotations.kubernetes.io/anything: "" => "blablah"

After

$ terraform plan
1 error(s) occurred:

* kubernetes_pod.echo: metadata.0.annotations: "kubernetes.io/anything" is internal Kubernetes annotation

[tombuildsstuff]

LGTM

[henning]

I'm stumbling into this because this solution makes implementing this way to configure a service impossible:

https://kubernetes.io/docs/concepts/services-networking/service/#ssl-support-on-aws

What exactly are the major problems for Terraform that occur if you let a user in his own responsibility change such a value?

I can see that if K8s resets such a value internally and automatically because it's not intended to be changed, Terraform will detect the change and write it again ans again on ech apply, but you already propose a solution for this with the ignore changes flag.

Apart from that, users can break stuff in their own infrastructure abusing the wrong internal annotations, but that's their own responsibility and that is valid for just about any configuration change.

I don't see why this must be so strict.
In the AWS Provider for example, TF accepts many inputs that are detected by AWS as wrong and then result in errors on apply that are not detected beforehand.

[bobbytables]

I really think this should be reverted. @henning makes the biggest points but in reality this restricts this provider more than it helps it. What other ways can we solve the original bug listed in #37 ?

[marranz]

Have you guys found any other way of implementing ssl on load balancers ?

I'm struggling with that now and to be honest, don't understand at all why we shouldn't be able to set internal annotations using TF

[simoneroselli]

I can understand "internal annotations" look too arbitrary code, but some of them are too essentials at this stage to be banned like this.

Why don't you just maintain a minimal set of annotations, as long as the situation is not clear? Of course I'm referring to the SSL support for the LoadBalancer type services (aws) and very few others.

I wouldn't see many options at the moment, except stop using terraform/kubernetes and deploy services via kubectl.

[lfshr]

@tombuildsstuff @radeksimko can this please be revisited? This completely kills the potential of using Terraform to spin out private K8s clusters. Happy to help in any way I can.

[oddur]

This breaks setting up a nginx ingress via a helm chart and routing services to it(https://github.com/helm/charts/tree/master/stable/nginx-ingress). For that, I need to set the kubernetes.io/ingress.class annotation.

Some of these internal annotation are quite valid to want to configure externally. Please revisit.

[sl1pm4t]

FWIW @alexsomesan I was able to workaround this issue in my fork, and allow support for internal annotations without causing a perpetual TF diff.
This is done by comparing the annotations read from Kubernetes against the configured annotations. If it's an internal annotation, but is configured on the resources then it is recorded in the state file, if not it is quietly dropped.
See
https://github.com/sl1pm4t/terraform-provider-kubernetes/blob/custom/kubernetes/structures.go#L103
and
https://github.com/sl1pm4t/terraform-provider-kubernetes/blob/custom/kubernetes/structures.go#L145

[alexsomesan]

@sl1pm4t @oddur Thanks for pinging me. I wasn't aware of this change (I joined way after this was done). I can look into options to allow back the lost flexibility. There may be some hope that TF 0.12.x could make this kind of stuff simpler to implement. I'll do some research.

[alexsomesan]

It just occurred to me - we might also be able to create a datasource where the template developer would specify a set of annotations as internal, and then aggregate the datasource's set with the one's from the resource.
Just a rough idea from 5 minutes ago - would that be of better use?

[pdecat]

@alexsomesan that would be awesome, I suggested something like that some time ago: #60 (comment)

Edit: in fact, I instead suggested allowing the white listing of annotations, as tracking internal annotations as kubernetes evolves would probably be harder at the provider level.

[dh-harald]

@alexsomesan Here's my POC for whitelisting... #60 (comment) Free to cherry-pick or reimplement it in your way

[rpatrick00]

@alexsomesan PR #244 contains the implementation of the @dh-harald POC from #60 (comment)

[kzap]

i agree this should be reverted.

[gwvandesteeg]

Well this should be definitely reverted, it causes more problems than it appears to have solved.
If you do really want to track the user defined annotations, you'll need a way to add the ones the user specified into the state file each time and it gives you the reference of which ones they actually care about, and then on apply/plan you do a diff on those defined in the current configuration and deal with the changes accordingly.

[hhamalai]

This is blocking e.g. the use of external-dns via terraform-provider-kubernetes, please revert or allow whitelisting annotations.

[cawilliamson]

Absolutely terrible idea - purge it with fire!

[mascah]

This needs to be reverted for all the reasons above.

[Starefossen]

WOW! Who thought this was a good idea?!

[tdmalone]

^ Update on all of the above - #325 has now been merged which should fix this issue!

[tdmalone]

This has now been fixed in 1.7.0: https://github.com/terraform-providers/terraform-provider-kubernetes/blob/master/CHANGELOG.md#170-may-22-2019
🎉 🚀