Epic: Allow for custom annotations & env variables on K8s Resources for Self-Hosted Installations

[lucasvaltl]

Summary

Allow users to customise to adapt their deployment of self-hosted to the specifics of the environment/cluster it is applied to.

Context

Customers are finding it difficult to sometimes use KOTS because it does not allow them to add custom annotations or env variables to K8s resources. The installer produces a fixed manifest and that then gets applied to the kubernetes cluster. Any changes made forces them to re-apply those changes after an update to Gitpod, which can lead to confusion.

Specific reasons they need to customise / make changes:

• Annotate service to use internal networking
• Add an environment variable to S3 when used as a registry storage backend (internal reference)

Value

Allow Kots to work in more setups, because it can be customised to environment specific circumstances.

Acceptance Criteria

• Customers can add generic annotations and env variables to resources via a configuration field in the KOTS UI.
• This is well documented in our documentation.

Measurement

• Even more customers start using kots

Proposed solution

Allow for generic annotations on resources - have a way to set this in the KOTS UI & Installer Config. Then apply together with the installation.

Alternatives considered

1. Open issues for the missing configuration options customers are doing currently with post-processing and solve this properly
2. Rely on replicated git-ops flow (requires a feature request from us) for post-processing.

Additional Context

• Likely related to Add Option to Set Proxy Service as ClusterIP Type in KOTS Admin UI #9981 and 0b599a8
• Examples of what customers are currently customising:

Tasks

• Apply dummy functions to all components #10853
• Allow for custom annotations #10826
• Allow for custom envvars #10902
• Allow for custom labels (stretch goal) #10903
• Allow customization of Helm components #10942
• Configure KOTS input #10910
• Validate the input object #10908
• Document for Installer #10840
• Document for website #10941

[corneliusludmann]

I doubt that we want to support post-processing in the near future. Instead, we should open issues for the missing configuration options customers are doing currently with post-processing and solve this properly. See also this internal comment.

[lucasvaltl]

Here is the current list of known example customisations that are being done by customers via post-processing,. which can partly now be done via experimental config OR which would benefit from having customisable annotations:

1. Change proxy service to a cluster IP instead of a loadbalancer type when using your own load balancer (this was changed via [installer] support service type ClusterIP for proxy #10537):

experimental:
  common:
    serviceConfig:
      proxy:
        serviceType: "ClusterIP" # case in-sensitive

2. Annotate proxy service with internal networking tag to force internal IP (was changed via 0b599a8)

experimental:
 webapp:
   proxy:
     serviceAnnotations:
        networking.gke.io/load-balancer-type: "Internal"

3. In S3, you need to add an environment variable for it to work as a registry because of old libs in distribution/registry.

echo "Gitpod: Add s3 endpoint"
    yq eval-all -i 'select(.kind == "Deployment" and .metadata.name == "registry").spec.template.spec.containers[].env += {"name": "REGISTRY_STORAGE_S3_REGIONENDPOINT", "value": "s3.eu-north-1.amazonaws.com"}' gitpod.yaml

4. Annotate ws-manager to allow for scraping not from promethues, but datadog (More info, internal)
5. if proxy has annotations for service.beta.kubernetes.io/aws-load-balancer-internal: 'true' as well as an annotation for the subnet the ELB should be in - it would save us from having to switch it to ClusterIp and for a customer to have to involve another team (source)
6. Set HTTP_PROXY env var (internal discussion)

[mrsimonemms]

This is superseded - ignore

I'm going to propose this as the new config structure:

experimental:
  annotations:
    common:
      key: value
    kind:
      <k8s_kind>:
        <component_name>:
          key1: value1

This would allow a common annotation on ALL components, but also fine-grained control of components based upon the kind (eg, deployment) and the component name (eg, ws-daemon). I've specifically added this as a separate config object (ie, not under experiment.webapp.kind.annotation) as this makes it a simpler, more isolated config.

I'm going to propose only applying this to deployments, daemonsets, statefulsets (and their pods) and also services. This will exclude the likes of jobs, networkpolicies, rolebindings and configmaps which I don't think will require much annotating.

Are we happy with this as a config definition and implementation? @lucasvaltl @corneliusludmann

[corneliusludmann]

I'm going to propose this as the new config structure

I think we should NOT put this under experimental since we want to support this officially. Or is there a good reason to have this first there and move it later?

Regarding the config object structure. Just a random idea. I would like a structure like this but it could be more difficult to parse. What do you think?

componentAnnotations:
  '*':
    '*':
      key1: value1   # all components
  services:
    '*':
      key2: value2   # all service components
  '*':
    server:
      key3: value3   # all components called 'server'
  services:
    'proxy':
      key4: value4   # only service called 'proxy'

Otherwise, I'm fine with your more simpler version of the structure. Maybe we can make annotations more specific and kind as opposed to common feels a little bit odd to me, however, I don't have a better idea.

[mrsimonemms]

Proposed Documentation

There are times when it is desirable for custom annotations, envvars and labels should be added to an installation.

Installer Config

This can be added to the root of the config.yaml. If nothing is passed in, no custom parameters are added in.

The structure is based upon the standard Kubernetes resource definition. For annotations and labels, these must match the apiVersion, the kind and the metadata.name - a wildcard * can be used to match all resources. For environment variables, the apiVersion and kind are ignored, as these are only implemented on containers. As before, the * wildcard can be used on metadata.name.

customization:
  - apiVersion: "*"
    kind: "*"
    metadata:
      name: "*"
      annotations:
        appliedToAll: value
        hello: world
      labels:
        appliedToAll: value
        hello: world
  - apiVersion: "apps/v1"
    kind: "Deployment"
    metadata:
      name: "ws-manager"
      annotations:
        hello: ws-manager
      labels:
        hello: ws-manager
    spec:
      env:
        - name: HELLO
          value: world

This example would generate the following spec (these are simplified for readability reasons):

---
# apps/v1/DaemonSet ws-daemon
apiVersion: apps/v1
kind: DaemonSet
metadata:
  labels:
    app: gitpod # system-value
    component: ws-daemon # system-value
    appliedToAll: value
    hello: world
  annotations:
    appliedToAll: value
    hello: world
  name: ws-daemon
---
# apps/v1/Deployment ws-manager
apiVersion: apps/v1
kind: Deployment
metadata:
  creationTimestamp: null
  labels:
    app: gitpod
    component: ws-manager
    appliedToAll: value
    hello: ws-manager
  annotations:
    appliedToAll: value
    hello: ws-manager
  name: ws-manager
spec:
  template:
    spec:
      containers:
        - env:
          - name: HELLO
            value: world

In the event of multiple matches, the final matching customization would be applied. For that reason, it is a good idea to structure your customization from least to most specific. System-generated values will never be overridden.

Validation

• All annotations and labels must be key/value pairs.
• All environment variables must be an array of envvars
• There must be a valid apiVersion, kind and metadata.name

KOTS Config

This is a raw YAML object that will be applied to the Installer config as-is - it will be a file upload (similar to the patch) as it's easier to create YAML files in an IDE than it is in a plain <textarea>. The format is described as above.

We will attempt validation with a preflight check. It's not currently clear how this will work, but should be possible

[lucasvaltl]

Thanks for writing this up @mrsimonemms ! Helps me understand how this would work in context I hope you found it helpful as well!

This raises a couple questions (happy to jump on a call):

• Is the way to write the config K8s standard or is it something we came up with? Context: just trying to understand how familiar users would be with this. Although granted it does not look complicated.
• What would our error handling look like, for example if someone were to have the wrong formatting?
• How would one apply this in KOTS? From the comments above, there would be a separate field (in the Kots admin ui) called installer config?
• This requires intimate knowledge of the components that Gitpod uses right? I think this is fine though - if you are doing custom annotations, you will know what you want to annotate.

[mrsimonemms]

@lucasvaltl I'll answer in order, but a call with you is always nice

• until you get the key/value pairs at the end, this is fairly custom. I took it from Cornelius's comment as I quite liked it - the * is a wildcard by convention in YAML
• error handling in KOTS would be limited - we can probably do something in a pre-flight check, but I'd have to investigate that before I commit to it. The Installer will error if it's invalid YAML
• KOTS will be a simple <textarea>. This has little-to-no validation in it, so it should be under the "advanced" section
• Yes, this is something that I foresee would only be done by either a) community members who like hacking or b) enterprise customers who have engaged us for support. Having it under the "advanced - here be dragons" section should reduce the button-pressers. In Kubernetes, annotations are generally ignored if not implemented, so it will only be an issue if they override something. This should be easily identifiable in a support bundle though.

[mrzarquon]

This also supporting labels would complete it as a feature for use by CS, different customers have different org requirements for how their workloads are deployed - some use labels to track things like hosting cost chargeback to the department that provisioned the app for example. The difference between Labels and Annotations is that Labels would need to be applied to all resources, not just a subset (but should also be harmless if we prevent users from changing the app and component keys, that gitpod uses itself).

Re: preflight, assuming the gitpod-installer is doing the evaluation of these customizations to apply them, wouldn't adding a installer --validate-customizations command to the installer which is then executed as a kots preflight (using run-pod) be sufficient? This ensures we're always using the same component to check the data hash as would be acting on it it.

But +1 from me for going in this direction

[mrsimonemms]

@mrzarquon that's one option. There is already a validation command in the Installer (installer validate config and installer validate cluster) but the problem is the reporting to the preflight checks.

tl;dr - I'll figure something out

[corneliusludmann]

• KOTS will be a simple <textarea>

What do you think about a file upload instead?

Reasoning:

• Writing YAML in a text field (that does not indent with <tab>) is quite cumbersome
• It's easier for us to provide customers with a file instead of something they need to copy in a text field properly

Drawback:

• You need to create that file in a separate text editor.

[mrsimonemms]

Good call @corneliusludmann. I'll change the docs comment to reflect that

[mrsimonemms]

Edit to docs

In the event that a Kubernetes resource has multiple annotations (eg, a Deployment has it on the Deployment spec and the Pod spec), the same annotation will be applied for both and can be overridden with the parent kind (in this example, deployment). Only Gitpod-generated annotations (eg, gitpod.io/checksum_config) will only be applied to the spec that they're required on.

[mrsimonemms]

Suggest to only support:

All components except:

• cluster
• gitpod

Resources to include - others are likely to have unintended consequences:

• jobs
• deployment
• daemon set
• stateful set
• config map
• services
• service account

[csweichel]

The format seems a bit cryptic. How about something that's closer to Kubernetes?

customization:
  - kind: Pod
    metadata:
      annotations:
        appliesToAllPods: "notice the omitted name"
  - kind: Pod
    metadata:
      annotations:
        appliesToAllPods: "will override the previous value"
  - kind: Service
    name: proxy
    metadata:
      labels:
        stage: production
        companyPolicy: foobar
      annotations:
        service.beta.kubernetes.io/aws-load-balancer-type: external
  - kind: Pod
    name: ws-manager
    metadata:
      labels:
        foo: bar
    spec:
      env:
      - name: DEMO_GREETING
        value: "Hello from the environment"
      - name: DEMO_FAREWELL
        value: "Such a sweet sorrow"

All applicable customisations are applied in order. Using the example above, for the ws-manager pod we'd end up with:

apiVersion: v1
kind: Pod
metadata:
  labels:
    foo: bar
  annotations:
    # whatever came from the installer
    appliesToAllPods: "will override the previous value"
spec:
  containers:
  - name: ws-manager
    env:
    # whatever came from the installer
    - name: DEMO_GREETING
      value: "Hello from the environment"
    - name: DEMO_FAREWELL
      value: "Such a sweet sorrow"

By default the spec section applies to the actual "service container".
If we ever need to customise env var of an init container, we can specify the name and match that one.

[mrsimonemms]

Thanks @csweichel. I like the approach and think there's a lot of merit in this, especially as it's idiomatic in the k8s space.

Two potential drawbacks I can see are:

1. it could get very big indeed, thus making it hard to read - especially problematic when ensuring that a skipped name or kind is correct.
2. it could lead people to think that we're supporting things we're not. I can imagine someone half-reading that, putting in a volume and then raising a bug ticket

I suspect that, because this is a feature request from customers, this may require several iterations before we get it to cater for everyone's needs. My suggestion would be that we go with the simple one in the v1 Installer config and keep this thought in reserve and consider implementing it when we start thinking about a v2 config definition.

WDYT @corneliusludmann?

[csweichel]

Thanks @csweichel. I like the approach and think there's a lot of merit in this, especially as it's idiomatic in the k8s space.

Two potential drawbacks I can see are:

1. it could get very big indeed, thus making it hard to read - especially problematic when ensuring that a skipped name or kind is correct.

Great point. Instead of omitting any of those fields, we could use * as per your original proposal. Omission of any of the fields would result in a config validation error.

2. it could lead people to think that we're supporting things we're not. I can imagine someone half-reading that, putting in a volume and then raising a bug ticket

If that happens there likely is a need for it - and then we can discuss what they're trying to solve and if this is the right way.

That said, I'm more worried about the small differences, e.g. the omission of the container name. Don't have a solution here, and agree that we're unlikely to get it right the first time.

My suggestion would be that we go with the simple one in the v1 Installer config and keep this thought in reserve and consider implementing it when we start thinking about a v2 config definition.

Frankly this sounds to me a bit like we're just going to push this problem out into the future. I'd hope that a v2 config is still reasonably far away, considering the breaking experience that would produce all around.

[corneliusludmann]

In PR #10857 the question has arisen from @aledbf whether we want to protect existing annotations/env vars/labels. As long as our existing customer use cases do not speak against it, I'm happy with protecting all existing ones in the CustomerOverride... functions and preventing overriding them. Should be pretty easy to respect in the implementation.

[mrsimonemms]

Edit: Updated the docs to reflect the spec that @csweichel proposed. Also made reference that system properties cannot be overridden.

[lucasvaltl]

Marking this as complete - @mrsimonemms is the outstanding issue (#10908) still relevant?

[mrsimonemms]

@lucasvaltl no, that's sort of a duplicate of the "check config" preflight - will close