Skip to content

Allow a variable to be declared "permitted but not required" #8

Description

@mikkoi

Allow a variable to be declared "permitted but not required"

Against main at Env::Assert 0.016. Line numbers and quoted code are
from the working copy, not from the CPAN release -- the released file is
perltidied and the repository copy is not, so they differ in whitespace.

Summary

.envdesc can only say "this variable must be present and must match". There
is no way to say "if this variable is present it must match, and its absence is
fine"
.

That gap makes --exact unusable in a container, which is the environment where
it would be most valuable.

Interestingly, the module already supports the concept internally -- it is
simply unreachable from a description file, and --exact overrides it. So this
is mostly about exposing something that already exists.

The problem, concretely

--exact reports every variable in the environment that the description does not
declare. In a hermetic process that is exactly what you want.

In a Kubernetes pod the environment is not yours. Kubernetes injects, before your
process starts:

  • KUBERNETES_SERVICE_HOST, KUBERNETES_SERVICE_PORT, KUBERNETES_PORT_443_TCP...
  • for every Service in the namespace, a family like MYSVC_SERVICE_HOST,
    MYSVC_SERVICE_PORT, MYSVC_PORT, MYSVC_PORT_5432_TCP_ADDR (unless
    enableServiceLinks: false is set, and even then the KUBERNETES_* ones
    remain)

plus whatever the base image sets -- PATH, HOME, HOSTNAME, often LANG.

With --exact on, every one of those is an error.

Why the obvious workaround is worse than the problem

You can silence them by declaring them in .envdesc. But a declared variable is
a required one, so:

# .envdesc
MYSVC_PORT_5432_TCP_ADDR=^.*$

now means this pod must not start unless that Service exists. Deleting an
unrelated Service from the namespace stops your application booting. You have
converted a cosmetic warning into a hard dependency on somebody else's resource,
which is a strictly worse outcome than the drift --exact was meant to catch.

There is currently no third option.

What already exists

Env::Assert::Functions::assert reads a per-variable required flag:

# lib/Env/Assert/Functions.pm:147
my $required = $var->{'required'}//1;

and the POD example at Functions.pm:85 passes required => 1 explicitly. So
the checking side understands optional variables.

Two things stop it being usable:

  1. The parser never sets it. file_to_desc turns a description line into
    { regexp => ... } and nothing else:

    # lib/Env/Assert/Functions.pm:247
    $desc{'variables'}->{ $LAST_PAREN_MATCH{name} } = {
        regexp => $LAST_PAREN_MATCH{value}
    };

    required is therefore reachable only by calling assert() with a
    hand-built data structure, never from .envdesc and never from the
    envassert command.

  2. --exact overrides it anyway:

    # lib/Env/Assert/Functions.pm:149
    if( ( $opts->{'exact'} || $required ) && ! defined $env->{$var_name} ) {

    With exact on, every declared variable is treated as required, even one
    explicitly marked required => 0. That is backwards: exact mode is precisely
    when you need to declare tolerated variables, and it is the one mode where
    declaring them cannot express toleration.

  3. The flag is also broken where it is reachable. Calling assert()
    directly with required => 0 and an absent variable falls through to the
    value check at line 157:

    elsif( $env->{$var_name} !~ m/$regexp/msx ) {

    $env->{$var_name} is undef there. Reproduced against main:

    $ perl -Ilib -e 'use Env::Assert::Functions;
        for my $re ( q{^.*$}, q{^[0-9]{1,}$} ) {
            my $r = Env::Assert::Functions::assert( {},
                { variables => { PORT => { regexp => $re, required => 0 } }, options => {} }, {} );
            printf "regexp %-14s success=%s\n", $re, $r->{success};
        }'
    Use of uninitialized value in pattern match (m//) at lib/Env/Assert/Functions.pm line 157.
    Use of uninitialized value in pattern match (m//) at lib/Env/Assert/Functions.pm line 157.
    regexp ^.*$           success=1
    regexp ^[0-9]{1,}$    success=0
    

    So an absent optional variable emits an uninitialized warning always, and
    then: with a permissive regexp it passes by accident (undef
    stringifies to q{}, which ^.*$ matches), and with a restrictive one it
    fails, reporting "invalid content" for a variable that is not there.

Points 2 and 3 are bugs in their own right and could be fixed without any new
syntax. Together they mean required => 0 does not currently work by any route
-- so this issue is less "add a feature" and more "finish one".

Proposed change

1. A way to mark a variable optional in .envdesc

The file already has a comment-tag convention (## envassert (opts: ...)), so a
per-variable tag is a natural fit. One suggestion:

# Required: absent or non-matching is an error
SMTP_HOST=^smtp-relay[.]example[.]com$

## envassert (var: optional=1)
LANG=^.*$

Alternatives worth considering, and I have no strong preference -- the
maintainer's taste should decide:

  • a marker on the name: LANG?=^.*$
  • a distinct separator: LANG~=^.*$
  • a tag comment on the preceding line, as above

Whatever is chosen should be hard to write by accident and should not break
any existing .envdesc, where ? and ~ are currently legal in a name.

2. Make --exact respect it

Change the condition at Functions.pm:149 so that a variable's own required
flag is honoured whether or not exact mode is on. Exact mode should govern "is
this variable known?"
, not "must this variable be present?" -- they are
different questions and it currently conflates them.

Resulting behaviour

declared, required declared, optional not declared
present, matches pass pass error only with --exact
present, does not match error error error only with --exact
absent error pass pass

The two bold cells are the new behaviour.

Acceptance criteria

  • A .envdesc can mark a variable optional, and envassert exits 0 when that
    variable is absent.
  • The same variable, when present with a non-matching value, is still an error.
    (An optional variable is not an unchecked one -- this is the case most likely
    to be missed.)
  • With --exact, an optional declared variable that is absent is not an
    error.
  • With --exact, a variable that is present and not declared at all is still an
    error, as today.
  • Existing .envdesc files behave exactly as before. Absent a marker, a variable
    is required.
  • No uninitialized warnings. An absent optional variable must skip the value
    check rather than matching its regexp against undef. Running the test suite
    with warnings fatal would catch a regression here.
  • assert() called directly with required => 0 behaves the same way as a
    description file that marks the variable optional.

Where to look

  • lib/Env/Assert/Functions.pm
    • file_to_desc (line 209) -- parses .envdesc; the variable-line branch at
      line 247 is where a marker would be recognised
    • _interpret_opts (line 257) -- existing tag parsing, worth reading for
      style before inventing a new one
    • assert (line 133) -- the presence check at line 149 and the exact-mode
      loop at line 166
  • lib/Env/Assert.pm -- assert_env (the exact option is handled around
    lines 108 and 128)
  • POD in both files, plus the envassert command's --man, which documents the
    .envdesc format and would need the new syntax

Tests

Run them with prove -l, per CONTRIBUTING.md. exact is already exercised in
t/env-assert-private.t and t/env-assert-public-assert.t, which are the
natural places to extend; the envassert command has its own tests in
t/envassert-script.t.

Worth covering explicitly, because the interaction is the whole point:

  • optional + absent + --exact -> pass
  • optional + present + bad value -> error
  • optional + present + good value + --exact -> pass
  • undeclared + present + --exact -> error (unchanged)
  • required + absent -> error (unchanged)
  • an existing description file with no markers -> behaviour unchanged

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions