Review: secret override flag when using YAML file

[viveksyngh]

In a stack.yml file if we have more than one function and one of the function uses secrets then it is being used for other functions as well.

Expected Behaviour

Ideally secrets should be used for only those function in stack.yml which has secrets defined when we deploy the functions.

Current Behaviour

secrets is used for the other functions in stack.yml even if they do not have secrets in function configuration.

Possible Solution

Steps to Reproduce (for bugs)

1. try to deploy openfaas-cloud without adding derek-private-key secret to either swarm or kubernetes

Context

This fails to deploy the function even if they do not use any secrets.

Your Environment

• FaaS-CLI version ( Full output from: faas-cli version ): 0.6.4

• Docker version ( Full output from: docker version ): 18.03.0-ce-rc4

• Are you using Docker Swarm (FaaS-swarm ) or Kubernetes (FaaS-netes)? Kubernetes

• Operating System and version (e.g. Linux, Windows, MacOS): MacOS

• Link to your project or a code example to reproduce issue:

[ivanayov]

Derek add label: proposal

[ivanayov]

@viveksyngh Thanks for proposing and working on this.

Can you please test with the latest 0.6.8 faas-cli version? (0.6.4 is a little old now)

[ivanayov]

IMO it's better if a secret is applied to all functions by default and the updated behaviour to be optional. Otherwise one will need to specify it for every function instead of configuring it once.

Any thoughts? @alexellis @johnmccabe @ericstoekl @rgee0

[alexellis]

Hi I agree that this is a confusing behavior - we should apply it if there is only a single function, to all (existing) or to none (showing a warning).

[rgee0]

My thoughts are that the behaviour needs to be deterministic. At the moment, if the backend is missing a secret, the same stack file containing a function referring to the missing secret will deploy different functions / fail with different functions deployed on repeated runs. Debugging this would be quite challenging and would require a level of understanding of golang data structures.

Additionally, even without missing secrets, there's an inherent inconsistency owing to the application of secrets being cumulative, so the first function with a secret will have one mounted, whereas the nth function will have n secrets mounted.

Agree with Alex that this needs a design review before progressing.

[alexellis]

@viveksyngh Thanks for raising an issue - this does need our attention so that we can figure out how to change or document the design. I'm going to close the PR since it is not the desired design.

A secret can be set for a manual override only when a YAML file is not being used

Or the YAML file only contains a single function or a --filter was passed which only matches a single function - both valid use-cases for an override of a secret.

Other than that this is a confusing behavior and we could exit with an error like:

A secret override cannot be passed when deploying from a YAML file.

While the issue is raised - should this apply to any other flags we can override?

[rgee0]

As I understand the issue:

A stack file has 100 functions detailed. 50 of them have a unique secret specified. The remaining 50 have no secret. One of the secrets hasnt been added to the chosen orchestrator.

The first run of faas-cli deploy sees 10 functions deployed. All 10 are from the set with secrets so fn1 has one secret and fn10 has 10 secrets. The attempted 11th function is the one with the missing secret. The set of secrets now contains the missing secret which causes the remaining 90 functions to all fail, including those without a secret defined.

The second run of faas-cli deploy shows entirely different behaviour without anything being changed in my config. This time I see 50 secretless functions deployed and fn51 is the one with the missing secret. The remaining 50 fns will not deploy.

The third run of faas-cli deploy, again with no changes to my config shows 49 of my functions with secrets deployed. The 50th is the one with the erroneous secret. It fails and so do the 50 secretless functions.

Running a fourth time sees the missing secret function arrive last so I find myself with 99 functions deployed.

As a user where do I start in understanding this behaviour? I havent used an over-ride flag and even if had I should expect a message detering use because im deploying from file.

[alexellis]

I'm not following the example you've given. Are you saying that a secret is attached some of the time but not the rest?

[viveksyngh]

I think it depends on the order in which functions are processed after loading from file. When we load functions definitions from file, every time it does not preserve the same order. That's why it has different behaviour everytime we run faas-cli deploy.

[rgee0]

1) It iterates a map which doesn't retain an order. 2) Secrets are cumulative so which secrets are applied depends on where they arrive in the order. 3) If a secret is missing then functions will fail from the point that the missing secret is attempted. 4) Due to 1) the point of failure due to 3) will vary from run to run

[rgee0]

This kinda caught us out this week in that one day we had a working system that actually shouldn't have been working and the next we didn't have a working system without us having changed anything.

[alexellis]

I need a bit more context, can you give an example of how this caught you out?

[rgee0]

We rebuild every morning. When deploying we had a secret that was required by a function but wasn't specified in the stack.yml. 9 out of 10 times deployments saw the secret mounted into the function (because of the scenarios already described), the 10th time it wasn't.

[alexellis]

I'm reading through the thread but it would help if someone could sum this up. Why does deploying the same code fail some of the time?

[rgee0]

The CLI collects secrets cumulatively. Imagine two functions that both make use of secret1, secret2 & secret3 and the incorrectly specified pseudo-yaml:

fn1:
secrets:
 - secret1
 - secret2
fn2:
secrets:
 - secret1
 - secret2
 - secret3

It's incorrect as fn1 should have the same secrets specified as fn2.

On the first deploy fn2 is processed first which means the CLI secrets slice will consequently contain secrets 1-3, which are subsequently mounted into fn1 when it is deployed.

On a different deploy the opposite is true and fn1 comes first. The function will deploy because secret1 and secret2 exist within the orchestrator, but the function code will barf when it encounters the reference to secret3 that hasn't been mounted.

Now scale this to a dozen or so functions and there stands a good chance that you wont experience the failure for an extended period of time.

[alexellis]

That makes a lot of sense said this way. Please can you create a new issue and link to this one? Then raise a PR when you're ready.

[alexellis]

/lock: stale