environment vars wan't be removed from runtime

[toschneck]

When an configuration value will be removed from func.yaml, it will be not removed from the container environment context after a new deployment.

how-to-reproduce:

1. add config val to func.yaml

name: jhello
version: 0.0.24
runtime: java
cmd: com.example.fn.HelloFunction::handleRequest
build_image: fnproject/fn-java-fdk-build:jdk9-1.0.56
run_image: fnproject/fn-java-fdk:jdk9-1.0.56
format: http
config:
  MY_ENV_VAL: test

and print out the envs e.g. in the java FDK:

package com.example.fn;

import java.util.stream.Collectors;

public class HelloFunction {

    public String handleRequest(String input) {
        return System.getenv().entrySet().stream().map(e -> e.getKey() + "=" + e.getValue()).collect(Collectors.joining(", "));
    }

}

2. deploy the function fn --verbose deploy --app japp --local
3. curl http://localhost:8080/r/japp/jhello response contains value MY_ENV_VAL=test
4. remove MY_ENV_VAL: test from func.yaml
5. deploy the function again: fn --verbose deploy --app japp --local
6. curl http://localhost:8080/r/japp/jhello response contains STILL the value MY_ENV_VAL=test

[denismakogon]

This is some kind of expected bahaviour. If run deploy more than once you tried to update func config with nothing, so nothing changed. You have to use CLI config API.

[toschneck]

@denismakogon In my opinion removing a value is an explicit config change, so I change something to unset. The current default behavior is also some kind of risky if e.g. you set some secrets into it. Everybody would expect if I remove the value from the config, it would be also removed in the environment of the re-deployed container. Also if I re-deploy the hole fnserver, the values are still present and will be set. In my opinion the fn.db shouldn't hold "deleted" values.

[treeder]

You should be able to set the value to empty string to remove it or use CLI/API fn apps config unset myapp MY_ENV_VAL. There's some previous issues on here about the same thing so it's been discussed before.

[rdallman]

here's one of the previous issues: #707 (comment)

it seems that k8s has inculcated the idea that a yaml config is definitive and not a delta, this is the 3rd time this has come up, we likely need to do something to ameliorate from current behavior since it seems not what users expect.

[treeder]

Ya, I'm coming to the same conclusion. For most things, I think it's OK to be a PUT, but this one case in particular, configs, is a different beast. A lot of configs will not be in the func.yaml since they are intended to be secrets and they change for every deployment. The ones you set from the CLI or via the API. How do we reconcile that?

[rdallman]

one part, seems we could have much stricter enforcement of func.yaml client-side. it's solely a CLI construct, the API itself we have some flexibility. what I mean here is that a func.yaml would contain all required fields (e.g. if a user dropped the line 'format: http' they'd get an error, not drop back to 'default' or ignore this field to keep the server's current value) and any non-required fields we would treat exactly like a PUT where if they're there, we put this in exactly, and otherwise we delete the field wholesale. as @treeder notes, config does not even have an option to behave like this at present, since each value must be specified with an empty value in order to be deleted from an existing config -- part of solving this may be to have this option in some capacity.

A lot of configs will not be in the func.yaml since they are intended to be secrets and they change for every deployment.

I agree to some extent, the idea of having to put every key:value in the func.yaml to carry around would mean that func.yaml are no longer 'github' safe for many production use cases, and have to be treated differently. operationally, the ops people are accustomed to this and have various secret management strategies. it does seem like we're trying to solve both configuration and secret management here, maybe the former is what we should be solving for, simply remaining flexible to the latter [vs solving for it] (i.e. configs may be constructed with secret management but secret management is not config management; trying to say they're mutually exclusive).

[toschneck]

In my opinion func.yaml still just can overwrite the default values, if nothing is set in format it will be fromat: http. BUT the config key-value pairs should be have the behaviour like other orchestration tools do. Like in docker-compose it would be also a possibility to override the secrets defined in func.yaml with a func.prod.yaml.

[carimura]

ya func.yaml is nice to solve for configuration, and definitely shouldn't for secrets (because checked in). orthogonal conversation, I like using the "expects" section to explicitly declare the secrets that are required. we definitely need a proper way to ensure all required things are made explicit to enforce good contracts.

anyways -- I keep running into this so +1 to deleting the config field if removed. assume technically we can make it work cleanly.