Sensu-go: Replicating a resource across multiple namespaces requires manual editing

[cwjohnston]

Expected Behavior

As a Sensu implementor or operator, I can write a resource definition to a file and apply it to more than one namespace without modifying the file content.

Current Behavior

Whether read from a file or STDIN, the namespace attribute of a resource must be modified for each intended namespace.

Possible Solution

Allow sensuctl --namespace flag specified on the command line to override the value of the namespace attribute as specified in input, whether read from file on disk or STDIN.

Context

In many cases operators will want to have some number of checks shared across every namespace. Currently, for deploying a single check to multiple namespaces, operators need to either make a copy of the check and modify it for each namespace, or otherwise script this approach.

The possible solution provided here would make this much less burdensome. For example, imagine I have a directory containing a collection of resource definitions in yaml and I would like to apply all of these resources to namespaces foo, bar, baz and qux

for n in foo bar baz qux 
do
  sensuctl namespace create $n
  cat **/*.yml | sensuctl create --namespace $n
done

Your Environment

• Sensu version used (sensuctl, sensu-backend, and/or sensu-agent): 5.3.0
• Installation method (packages, binaries, docker etc.): packages

[calebhailey]

+1000

[portertech]

We should allow namespace omission in definition/manifest files, use the current sensuctl default namespace, and allow for an --namespace override. If a definition/manifest file specifies a namespace, there should be no way to override it.

[annaplotkin]

I like this proposal 👌

[palourde]

The solution I came up a few weeks ago:

Basically, before adding it the resource to the list of resources, check if the namespace exists (https://github.com/sensu/sensu-go/blob/760d1e2f77adadf9585598121e224b3c9bc12ce3/cli/commands/create/create.go#L124)

If it doesn’t, use the current namespace.

If it does, check if the namespace flag was actually used (https://github.com/sensu/sensu-go/blob/760d1e2f77adadf9585598121e224b3c9bc12ce3/cli/commands/helpers/flags.go#L54) and if so, override whatever was defined in the resource.

[ccressent]

I personally don't think this sort of things should be implemented in sensuctl. Today it's about namespaces, which seems reasonable. Tomorrow it's going to be about subscriptions. The day after, about yet another field. Before long, you're implementing everybody's pet use case in your tool, making it complex to use and maintain.

Instead, I advocate for the Unix way: a program should do one job and do it well; compose programs to build more complex operations. Here is how I would solve this issue, without changing sensuctl:

for n in foo bar baz qux 
do
  sensuctl namespace create $n
  cat **/*.json | jq ".spec.metadata.namespace = \"$n\"" | sensuctl create
done

If you're using yaml, convert it to json before piping it into jq, or use yq which does that for you.

Regardless of my stance on this, if we do implement that --namespace flag in sensuctl create, people who haven't upgraded to the right version of sensuctl will still need to use the jq/yq approach. So hopefully the snippet above helps you @cwjohnston!

[palourde]

That's a valid point (about the Unix philosophy) but I believe in this particular instance the UX is far more important.

One of the current challenge is that if you print the help usage of the sensuctl create command, we show the --namespace flag as available. In fact, I believe almost all commands support this flag so as a user, I would expect it to be available for this particular subcommand too.

But more than that, I feel like it's a super neat functionality for operators. Hopefully most users store their Sensu resources files in version control (which is a common best practice). Therefore, not having support for specifying a namespace at runtime would basically mean a lot of files would have to be duplicated just to define the namespace. We also assume that all operators will have jq installed, or even yq, and this is an error prone process.

Implementing this functionality would allow operators to have _global_ resources in their version control so there's only a single file to maintain. For example, you could have a single RoleBinding file that refers to a ClusterRole, so whenever you bootstrap a new namespace, you only have to sensuctl create that file instead of creating a new file.

For reference, here's the kubectl implementation which I really like. It differs a bit from what we discussed though, but here's the TL;DR version:

• You can write a resource file without specifying a namespace. When you kubectl create it:
  
  
  
  1. It first verifies if the --namespace flag was specified. If so, it uses that namespace
  
  
  2. In case no flag was pass, it uses your current namespace.
  
  

If you specify a namespace in the resource declaration, the resource will always be created in that namespace. If you try to use the --namespace flag to set another namespace, the command will fail.

[annaplotkin]

@palourde @nikkiki @ccressent - I have pondered this since last night and I am in agreement with Simon. I would like to move forward with that implementation suggestion. Please let me know if you have any questions!

cc @portertech for transparency

[ccressent]

The decision to go forward with this has been made, and I'm OK with that, but I still want to respond to @palourde's comment with my thoughts in an attempt to better explain my position.

we show the --namespace flag as available [...] I would expect it to be available for this particular subcommand too

It is available, but it just displays a warning about it being useless if you use it. I think it would be equally valid in that case to just remove it.

Hopefully most users store their Sensu resources files in version control (which is a common best practice). Therefore, not having support for specifying a namespace at runtime would basically mean a lot of files would have to be duplicated just to define the namespace.
[...]
Implementing this functionality would allow operators to have global resources in their version control so there's only a single file to maintain.

With the approach I outlined, you can still (and should) keep your resources in version control. They don't need to have a namespace specified either; jq will add it if it's missing and modify its value if it's already there. No resource duplication. You let jq do the "templating". And that works for _any field_, not just namespaces.

As an aside, that's just scratching the surface of what jq can do. You could also write expressions that set values conditionally, or computed based on other fields in the object, ... That relates to my point about not having to consider and support everybody's pet use case in sensuctl. jq (or anything else that fits your use case) can do it better, upstream of sensuctl.

We also assume that all operators will have jq installed, or even yq, and this is an error prone process.

I agree that less dependencies is better, but I would argue that an operator of a sensu cluster ought to be able to install a program on a single machine (the one with sensuctl) in order to support their advanced use case. Especially if it's a tool that, like jq, is part of most distributions' package collection, which is not even the case of sensuctl.

It doesn't have to be jq/yq either. Whatever fits your need. You could do simple tasks with sed for example. And again, if you don't need to "template" your resources, you don't need to install additional tools.

All of that being said, supporting that --namespace flag for create won't conflict with what I'm describing. Advanced operators can still do what I'm suggesting, regardless of that flag being available or not. My whole point was more that it's not _absolutely needed_, and that the less features we support, the less code we have to maintain and debug! :)

Regarding kubectl's approach, I think it's sound and we could do the same thing.

[palourde]

I'm realizing this should probably be done at the API level so we can also support use cases like https://github.com/sensu/sensu-go/issues/2716.

[nikkictl]

@palourde I've been thinking about this approach a little bit and have some preliminary concerns. First, I think #2716 should remain separate from this issue. The sensuctl create command only talks to the API through the generic client. This behavior should remain generic, its only purpose being to PUT a resource at its inferred URI path. At this point, the namespace should have already been injected into the resource (by sensuctl) so the client can infer the URL path and create the resource at that path. Additionally, the namespace for both of these issues are captured by 2 different mechanisms. Sensuctl gets the namespace from the flag/config/resource declaration, while the API gets the namespace from the URL. We'll have to special case all non-namespaced resources on the API side to achieve that behavior. TL;DR although the problems seem similar, I believe the solutions are unrelated.

Secondly, I'm a bit confused by the following behavior:

If you specify a namespace in the resource declaration, the resource will always be created in that namespace. If you try to use the --namespace flag to set another namespace, the command will fail.

Essentially, this would mean that _all_ resources in a file must be consistent if a namespace is declared or not?

I believe we have some other use cases we should cover. The proposed approach (outlined below) could produce a clunky UX under certain conditions, and has somewhat contradictions in the behavior.
1. Resource declarations with no namespaces defined, --namespace flag is set
- The namespace provided by the flag is injected into all resources.
2. Resource declarations with all namespaces defined, --namespace flag is set
- The command will fail.
3. Resource declarations with some namespaces defined, --namespace flag is set
- Does the command succeed for resources without a namespace declared, but fails for resources with a namespace declared?
4. Resource declarations with no namespaces defined, --namespace flag is unset
- The current namespace of the sensuctl config is injected into all resources.
5. Resource declarations with all namespaces defined, --namespace flag is unset
- The resources are created as declared.
6. Resource declarations with some namespaces defined, --namespace flag is unset
- Does the command create resources as they are declared, but inject the current namespace if one is not declared for a resource?

Perhaps we should pick a new approach that yields more concrete behaviors. For example, the namespace flag overrides any configuration or resource declaration. If a namespace flag is not set, the current configured namespace overrides the resource declaration. This aligns with the intended behavior described in #2716.

[nikkictl]

I believe I have a solution for the behavior described here: https://github.com/sensu/sensu-go/issues/2826#issuecomment-490587218

Order of precedence:

1. resource declaration
2. --namespace flag
3. current sensuctl namespace configuration

[palourde]

I think #2716 should remain separate from this issue.

I believe it should actually be solved in the same manner in order to avoid duplicating business logic into the API and sensuctl; most of this logic should live in the API for consistency. The idea is simply that sensuctl should be responsible for sending the payload (that may or may not contain a namespace) to the right URL, which needs to contain a namespace, which can all be done with the generic client. Meanwhile, the API should be responsible to ensure the namespace in the payload corresponds to the one in the URL; either add it if it's missing or reject the request if they do not match.

Essentially, this would mean that all resources in a file must be consistent if a namespace is declared or not?

No, because we loop though all resources to determine their namespaces, we do not choose a single namespace for _all_ resources, see: https://github.com/sensu/sensu-go/blob/f94d79afe232d607a65d203dd85bb569ea4fd954/cli/commands/create/create.go#L182 &
https://github.com/sensu/sensu-go/blob/5ffdd077e8df24d76bafa769577fa78f5cb8e3cd/cli/client/generic.go#L109

I believe we have some other use cases we should cover. The proposed approach (outlined below) could produce a clunky UX under certain conditions, and has somewhat contradictions in the behavior.

Here's the UX I had in mind:

1. Resource declarations with no namespaces defined, --namespace flag is set
   The namespace provided by the flag is injected into all resources.

This is correct. Resources will be created in the namespace specified by the flag.

Resource declarations with all namespaces defined, --namespace flag is set
The command will fail.

Actually it should only return errors for the resources that specified a namespace different than the one specified by the flag.

1. Resource declarations with some namespaces defined, --namespace flag is set
   Does the command succeed for resources without a namespace declared, but fails for resources with a namespace declared?

Yes, since we do an HTTP request for each resource, we should create the one we can and return errors for the one that failed to create because they had a different namespace than the one specified in the flag; we should not fail if the resource specifies the namespace dev and --namespace dev is specified.

1. Resource declarations with no namespaces defined, --namespace flag is unset
   The current namespace of the sensuctl config is injected into all resources.

That's right.

1. Resource declarations with all namespaces defined, --namespace flag is unset
   The resources are created as declared.

👍

1. Resource declarations with some namespaces defined, --namespace flag is unset
   Does the command create resources as they are declared, but inject the current namespace if one is not declared for a resource?

It goes back to 4. and 5.; we use the namespaces in the resources if defined, otherwise we fallback to current sensuctl namespace.

Let's schedule a meeting maybe if there's still any confusion?

[nikkictl]

Yes, I'll set up a discussion. What concerns me with the approach is that we infer the namespace URL from the declared resource's URI path: https://github.com/sensu/sensu-go/blob/5ffdd077e8df24d76bafa769577fa78f5cb8e3cd/cli/client/generic.go#L109

Again, see https://github.com/sensu/sensu-go/issues/2826#issuecomment-490587218. The command should not fail under any circumstances, rather we follow an order of precedence like in https://github.com/sensu/sensu-go/issues/2826#issuecomment-497486371.

[portertech]

Moving forward with the three levels of precedence. Fairly easy to understand and it allows for multiple resource definitions with and without specified namespaces to share a single file.

[calebhailey]

Actually it should only return errors for the resources that specified a namespace different than the one specified by the flag.

This is important and aligns with the K8s behavior (if a namespace in the file and the flag don't match, raise an error).

[palourde]

@calebhailey So we actually had a discussion last Friday about that and Sean expressed his preference for sticking with the order of precedence instead of returning an error. If I remember correctly, the argument was that you should be able to mix _global_ resources with namespace-scoped resources within a single file without returning errors.

I don't have any strong preference so feel free to assemble the Monitorama crew and discuss the matter!

[nikkictl]

[portertech]

@nikkiki @palourde @calebhailey both approaches have benefits and we can always change the behaviour. Aligning with the kubernetes approach just limits what you can do w/ a single file. I am still inclined to go the more permissive route as an experiment.