kubernetes: Ignore internal K8S annotations in config_map + use PATCH

[radeksimko]

This is something you'd unlikely noticed in the context of config map since config maps aren't annotated internally by default, but some other resources which I'm going to PR very soon are and I'd like to have the code in place and reviewed in isolation before adding more LOC.

Why ignoring internal annotations

Each K8S object can have annotations which can be defined by the user, but more often are used by tools that interact w/ the K8S API - also by kubelet internally. Kubelet is maintaining annotations like these:

• pv.kubernetes.io/bound-by-controller = yes
• pv.kubernetes.io/bind-completed = yes

et cetera. The convention and expectation from a K8S API consumer is to preserve any existing ones that it doesn't manage - therefore Terraform should not be deleting any existing annotations (which is what it does now).

Should Terraform have dedicated annotations prefix, e.g. terraform.io/?

I don't know, maybe? I'm not sure where is the right balance between allowing people to do anything, exclusively w/ Terraform VS allowing them to use Terraform alongside other tools with may be annotating resources too.

Looking at some other tools the approach really differs:

• https://fabric8.io/guide/annotations.html (strictly separated)
• https://www.projectcalico.org/calico-network-policy-comes-to-kubernetes/ (generic, i.e. policy)

Feedback welcomed. If we decide to have our own namespace, it would mean BC, but it's early enough in the life of the K8S provider, so should be pretty painless for most users.

Why PATCH

While Update()s seem to be working fine, it's merely because kubelet is watching for any changes in the cluster and re-associates any annotations as necessary (you're able to watch how this happens in a matter of seconds via kubectl get -w). We do not want Terraform to stamp over other APIs or Kubelet annotations in any way.

[stack72]

@radeksimko the code looks sound - got a few small questions within

As it's well tested, the code looks to work as expected

[stack72]

Nit pick - should this file really be called patch_operations? It seems there are patch and replace operation funcs within :)

[radeksimko]

So the struct type is called PatchOperation and the word patch comes from PATCH HTTP method, or more specifically from JSON Patch - so I think it's the right name.

I think eventually we could use these helpers cross providers, but it depends on the exact implementation of the SDK.

[stack72]

Are we guaranteed to have metadata.0. ?

[radeksimko]

Good question, I believe we are:

&schema.Schema{
		Type:        schema.TypeList,
		Required:    true,
		MaxItems:    1,

[stack72]

In that case, LGTM

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.