Page MenuHomePhabricator

Relax restrictions on toolforge envvar names
Open, MediumPublicFeature

Description

Feature summary (what you would like to be able to do and where):
toolforge envvar names are currently limited to the regex ^[A-Z_][A-Z_0-9]{3,}$. Please make it more permissive – ideally as much as the underlying Kubernetes API allows (see below).

Use case(s) (list the steps that you performed to discover that problem, and describe the actual underlying problem which you want to solve. Do not describe only a solution):
I’m working on migrating the QuickCategories tool to read its configuration from environment variables rather than config.yaml, with the goal of porting it to the build service, so I can ultimately run the background runner as a continuous job with a health check and thus hopefully fix T374152. To read the config from the environment, I am using Flask’s config.from_prefixed_env(), which supports reading nested objects from variables with double underscores in their name. For example, the current config.yaml snippet

EDITGROUPS:
    commons.wikimedia.org:
        url: "https://editgroups-commons.toolforge.org/b/QC/{0}/"
        since: 2021-09-14T00:00:00Z

would correspond to the environment variables

TOOL_EDITGROUPS__commons.wikimedia.org__url='https://editgroups-commons.toolforge.org/b/QC/{0}/'
TOOL_EDITGROUPS__commons.wikimedia.org__since='2021-09-14T00:00:00Z'

As far as I can tell from the Kubernetes documentation (1.29, latest), this should be supported in a secret:

The keys of data and stringData must consist of alphanumeric characters, -, _ or ..

However, toolforge envvar enforces additional restrictions on the keys, which makes the above variable names unavailable.

tools.quickcategories@tools-bastion-13:~/www/python/src$ python3 -c 'import yaml; print(yaml.safe_load(open("config.yaml"))["EDITGROUPS"]["commons.wikimedia.org"]["url"])' | toolforge envvars create TOOL_EDITGROUPS__commons.wikimedia.org__url
Error: {"message":"request body has an error: doesn't match schema #/components/schemas/Envvar: Error at \"/name\": string doesn't match the regular expression \"^[A-Z_][A-Z_0-9]{3,}$\""}

I don’t know why it does this; the documentation currently claims that “the name for the envvar has to be all caps, just like a bash environment variable” (emphasis added), but this is simply not true: bash environment variables can have any case.

Benefits (why should this be implemented?):
Tools would be able to use environment variables with fewer arbitrary restrictions.

Event Timeline

I looked a bit through the Git history but didn’t find a lot more explanation for the pattern.

The last task mentions that “We introduced a bit stricter check for the envvar names”, but doesn’t explain why.

Mentioned in SAL (#wikimedia-cloud) [2024-09-15T19:25:52Z] <wmbot~lucaswerkmeister@tools-bastion-13> deployed 377aab02c6 (lowercase nested keys to work around T374780)

Lowercase might be ok to include yep, though . and - are not valid bash variable characters, so that would not be possible (even though they are valid k8s secrets, as we expose the secrets as environment variables in the shell, not as mounted files).

I think that the wiki text was just a bad wording (it has to be all caps, and also all the bash envvar restrictions, just reworded it).

Do we have to be bound by Bash’s syntax limitations? . and - work just fine between env and Flask / Python, even if there is a Bash in between:

$ env 'TOOL_a.b-c=d' bash -c python3
Python 3.12.6 (main, Sep  8 2024, 13:18:56) [GCC 14.2.1 20240805] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import flask
>>> config = flask.Config('')
>>> config.from_prefixed_env('TOOL')
True
>>> config['a.b-c']
'd'

So I was hoping that, as long as the environment variables aren’t set via Bash (but rather via e.g. setenv() in k8s’ code), those characters would work.

Mentioned in SAL (#wikimedia-cloud) [2025-07-12T17:16:09Z] <wmbot~lucaswerkmeister@tools-bastion-13> deployed 15261afefb (change config keys to uppercase to work around T374780)

Mentioned in SAL (#wikimedia-cloud) [2025-07-13T14:43:26Z] <wmbot~lucaswerkmeister@tools-bastion-13> deployed b0af29d932 (change config keys to uppercase to work around T374780)

Mentioned in SAL (#wikimedia-cloud) [2025-07-16T19:05:53Z] <wmbot~lucaswerkmeister@tools-bastion-13> deployed c9512a1c58 (change config keys to uppercase to work around T374780)

^ Let’s try the low-hanging fruit (already backed by T374780#10162297) first then, I guess.

Mentioned in SAL (#wikimedia-cloud) [2025-07-22T23:33:28Z] <wmbot~lucaswerkmeister@tools-bastion-13> deployed 5c0aff28d3 (change config keys to uppercase to work around T374780)

fnegri triaged this task as Medium priority.Jul 23 2025, 2:18 PM

Seems like it’s not so easy after all :( see this GitLab conversation.

As far as I can tell from the Kubernetes documentation, this should be supported in a secret:

The keys of data and stringData must consist of alphanumeric characters, -, _ or ..

There’s something else I missed at the time:

The name of a Secret object must be a valid DNS subdomain name.

And the documentation linked there mentions that “the name must […] contain only lowercase alphanumeric characters, '-' or '.'” (emphasis added). Toolforge actually maps between lower-kebab-case secret names and UPPER_SNAKE_CASE envvar names:

kubetools.lexeme-forms@tools-bastion-13:~$ kubectl get secrets
NAME                                              TYPE                                  DATA   AGE
default-token-xztsc                               kubernetes.io/service-account-token   3      5y226d
toolforge.envvar.v1.tool-oauth--consumer-key      Opaque                                1      16d
toolforge.envvar.v1.tool-oauth--consumer-secret   Opaque                                1      16d
toolforge.envvar.v1.tool-replica-password         Opaque                                1      678d
toolforge.envvar.v1.tool-replica-user             Opaque                                1      678d
toolforge.envvar.v1.tool-secret-key               Opaque                                1      16d
toolforge.envvar.v1.tool-toolsdb-password         Opaque                                1      678d
toolforge.envvar.v1.tool-toolsdb-user             Opaque                                1      678d
tools.lexeme-forms@tools-bastion-13:~$ toolforge envvars list
name                         value
TOOL_OAUTH__CONSUMER_KEY     ****************
TOOL_OAUTH__CONSUMER_SECRET  ****************
TOOL_REPLICA_PASSWORD        ****************
TOOL_REPLICA_USER            ****************
TOOL_SECRET_KEY              ****************
TOOL_TOOLSDB_PASSWORD        ****************
TOOL_TOOLSDB_USER            ****************

And this breaks down if lowercase variable names need to be supported too.

(Also, the link to the 1.27 Kubernetes docs broke in the meantime. Toolforge is on 1.29 at the moment, so I’ll update the link in the task description in a moment.)

Yep, I think that at this point we might want to introduce a 'storage' for configured envvars, instead of relying on the k8s objects to embed the info in them (similar to what we do with components/deployment and we are moving towards for jobs), that might require some refactoring though.