Document expected structure of Jsonnet output

[lawrencejones]

Hi team,

We're trying out tanka for managing our core kubernetes cluster configuration but have hit a few roadbumps.

Most of our problems appear to stem from lack of understanding about what type of object tanka is expecting your jsonnet to generate from the environments file.

From looking at your examples, it appears the implicit expectation is that tanka will recursively descend your jsonnet structure, assuming that your structure is a tree with kubernetes object leaf nodes. So that assumes that you have one global object which is a map of string to object, where that object is either a kubernetes resource or another tree.

data Resource = Resource {
  apiVersion :: String, kind :: String, metadata :: Metadata
}
data Environment = Resource || [(string, Environment)]

If this is so, it would be great to document it. I suspect we might be missing something though, as tanka panics when we construct several otherwise valid configs, such as:

{
  a: {
    apiVersion: 'v1',
    kind: 'List',
    items: [/* valid k8s resources */],
  },
  b: self.a,
}

Lacking a clear explanation about what this format is makes it difficult to adopt tanka as a tool. All the advertising says tanka is a great function, but we don't know what the type signature is.

Can you help us out? Cheers.

[issue-label-bot[bot]]

Issue-Label Bot is automatically applying the label kind/question to this issue, with a confidence of 0.89. Please mark this comment with :thumbsup: or :thumbsdown: to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

[sh0rez]

Huh, nice catch!! We are indeed expecting a good old JSON dict with the Kubernetes objects as leaf nodes:

{
  "foo": {
    "kind": "Deployment",
    "apiVersion": "apps/v1",
    "spec": {
    }
  },
  "bar": {
    "baz": {
      "kind": "Namespace",
      "apiVersion": "v1",
      "metadata": {
        "name": "baz"
      }
    }
  }
}

The keys don't matter, they are discarded anyways. We were using this pattern internally with ksonnet before.

You are right, we are clearly lacking documentation here :D

[lawrencejones]

Ok, this makes sense. I had a hunch that lists causing a panic was a bug rather than it being totally at odds with the methodology.

So just to make sure we're not doing anything really weird, we've been following a style as advocated by databricks to produce libsonnet files that can template out a $service, such as a prometheus, or a grafana.

These files look like this, where the individual resources are locals, and we output an api list.

This seemed compelling to us as we have a convention of namespacing all app components together, and wanted to have a good pattern for achieving this without forgetting to apply a namespace anywhere. By outputting a list, we can do this:

{
  local namespace = ...,

  apiVersion: 'v1',
  kind: 'List',
  items: [
    resource {
      metadata+: {
        namespace: namespace.metadata.name,
        labels: $.labels,
      },
    }
    for resource in [
      namespace,
      serviceAccount,
      ...,
    ]
  ],
}

In our top-level environments file, we then merge all these together like so:

local certManager = import 'cert-manager/cert-manager.libsonnet';
local externalDns = import 'external-dns/external-dns.libsonnet';
{
  gcpProject:: 'gc-lab-project-5e7b',
  certManager: certManager {
    gcpProject: $.gcpProject,
  },
  externalDns: externalDns {
    gcpProject: $.gcpProject,
    domainFilter: ['gocardless.io'],
  },
  ...
}

Obviously this panics given the issue with lists. We got things to work with a hacky PR https://github.com/grafana/tanka/pull/109, but it would be good to know if we're doing something odd or strange here.

Does this look like a valid use of tanka? Have we just got unlucky and hit a bug?

Thanks again for your help, it's really difficult to come across good examples of people using these libraries.

[sh0rez]

I am not sure, I don't see why this should panic right away ... A List is a valid Kubernetes object and having a List as a leaf node should work .. you will just receive the list in the final output of tk show

[sh0rez]

Can you share some details on the exact error?

[lawrencejones]

It seems tanka determines what is a kubernetes object by checking if it has apiVersion and kind. Once it's identified these objects, it then attempts to sort them by name, but a list has no name or metadata.

What seemed more natural was for us to expand kubernetes lists into the child resources, so flatten everything into the final output. This would be a more intelligent translation than just recursively stepping through the config structure though, so only makes sense if tanka wants to have an opinion on how to unpack your jsonnet, which is why I was keen to understand what the typing was meant to be.

[lawrencejones]

https://github.com/grafana/tanka/blob/master/pkg/kubernetes/kubernetes.go#L86-L91

That's the line that panics, which will only trigger if you have more than one list object.

[sh0rez]

Oh yeah, this is kind of naive to expect every object to have Kind and Name. I wasn't expecting List to ever occur, because it is not strictly a real Kubernetes object but rather a helper thing.

TLDR we should fix that code to just return false in metadata.name is missing on one of these objects.

The sorting is really not crucial to the logic but rather a UX feature to have a stable tk show output

[lawrencejones]

What is tanka's philosophy on how it should handle the jsonnet output? Should it be totally dumb, and trust it, meaning it's ok if the parsing panics rather than presenting a good error?

Do you think it should be dumb, and never try to intelligently extract kubernetes resources? Unpacking lists, for example, might make a lot of sense to most users.

I'm not sure how much the tanka core devs have a position on this, but it feels worth taking one.

[lawrencejones]

Haha just checked the contributor lists, and it appears you are the core dev. In which case, I'd advise you to have a position on this!

Kustomize has very strong opinions on what it should and should not do. I suspect tanka is more likely to see adoption if it can articulate its own opinions :)

[sh0rez]

Tanka should never ever panic, if there is an error it needs a good error message that helps the user debug.

Tbh, I haven't really thought about the List type so far, because I have not seen a lot of value in it. Still, your use-case seems valid and it makes sense to flatten it. Would that have any unexpected side-effects that might not be immediately obvious to the user? After all we are mutating THEIR objects here .. they give us a list and tk show would hide that fact.

So we could either go down the path you just proposed, or discourage outputting lists, as it disallows features like Targets.

This whole issue could btw be solved on the Jsonnet level but just flattening the list there ;)

[sh0rez]

Opinions? /cc @rfratto @malcolmholmes @tomwilkie

[rfratto]

I'm not speaking authoritatively here, but I also don't think we should support List. It doesn't appear to be an actual API object and I can't find any documentation for it. In general, I'm not sure I understand why you need the List at all in your example: just exposing the service and deployment objects as keys would be enough to be picked up as API objects, as @sh0rez had shown in his first example.

If it helps, you can see the pattern we've been using to define objects in Loki and throughout our jsonnet libs.

[lawrencejones]

but I also don't think we should support List

I'm also not sure about this. The main reason we were bundling things in a list, other than following the way databricks suggest you do things, was because we wanted to map the namespace across every object. If you have to remember to apply the namespace to each resource definition, it makes it very easy to accidentally miss one and send the resource to the wrong namespace.

It's entirely for the:

    resource {
      metadata+: {
        namespace: namespace.metadata.name,
        labels: $.labels,
      },
    }
    for resource in ...

Which is replicating the namespace and commonLabels transformers that you get from kustomize.

Is there an easier way to do this in jsonnet?

[sh0rez]

Yes! The syntax you are using is not specific to your List object.

Imagine you had a local called foo being an array of valid Kubernetes objects:

local foo = [ ... ];

resource + {
  metadata+: {
      namespace: „abc“,
      labels: {} 
    }
}

This would overwrite both namespace and labels for all objects in foo

This will work once #112 is merged

[sh0rez]

Also keep in mind that by omitting the namespace altogether, Tanka will set the namespace defined in spec.json onto those objects

[lawrencejones]

Ahhh, #112 is what I was missing. It was clear tanka couldn't support lists of resources, and objects in jsonnet aren't functors, in that you can use object comprehensions but they map dicts to lists rather than dicts to dicts, so mapping a namespace over all of a jsonnet modules values wasn't something I could figure out how to achieve.

If #112 supports lists of resources then we can happily ditch the List type in favour of our jsonnet libraries being structured something like:

{
  namespace:: error 'must provide namespace',
  ...,

  local namespace = ...,
  local deployment = ...,

  resources: [
    resource {
      metadata+: {
        namespace: namespace.metadata.name,
        labels: $.labels,
      },
    }
    for resource in [
      namespace,
      deployment,
      ...,
    ],
  ]
}

Also keep in mind that by omitting the namespace altogether, Tanka will set the namespace defined in spec.json onto those objects

Yep, unfortunately this doesn't work for us. We want to use jsonnet to define an environment per kubernetes cluster and stamp out all the core services we expect to be in the cluster. We separate each core service into its own namespace, so the concept of there being a 'default' namespace that tanka should use is a bit broken for us.

It's why I'm very keen to find a pattern that ensures every resource output by a jsonnet module is namespaced. Accidentally sending a couple of resources for a particular application into the default namespace is not ideal.