feat: add Kubernetes Secrets as environment variables to pipeline steps

[cordeirops]

Summary

Implements #873 — lets a pipeline step consume a Kubernetes Secret value
as an environment variable, instead of hardcoding credentials in the
notebook.

A new cell tag, secret:secret_name:secret_key:ENV_VAR_NAME, is parsed
and compiled to a kfp.kubernetes.use_secret_as_env(...) call for the
corresponding step — following the exact same tag / StepConfig /
Jinja2-template pattern already used for labels, annotations, and
resource limits.

Changes

Backend

• kale/processors/nbprocessor.py: new SECRET_TAG grammar
  (secret:<name>:<key>:<ENV_VAR>), parsed both as a per-cell tag and
  as a pipeline-wide steps_defaults entry.
• kale/step.py: new StepConfig.secrets field
  ({ENV_VAR_NAME: {secret_name, secret_key}}).
• kale/config/validators.py: new K8sSecretsValidator chain
  (EnvVarNameValidator, K8sSecretNameValidator, K8sSecretKeyValidator,
  K8sSecretRefValidator) validating the env var name and the
  Secret's name/key format.
• kale/templates/pipeline_template.jinja2: imports and calls
  use_secret_as_env(...) once per configured secret.
• Golden DSL fixtures (iris.py, pipeline_parameters_and_metrics.py)
  updated for the new import line.

Frontend

• New SecretsDialog.tsx: a SECRETS button on the cell metadata
  editor opens a dialog with a dynamic add/remove list of
  Secret Name / Secret Key / Env Var Name rows.
• TagsUtils.ts / useCellTags.ts / CellMetadataEditor.tsx /
  InlineCellsMetadata.tsx: parse, serialize, and thread the new
  secrets field alongside the existing limits/baseImage/
  enableCaching state, so it survives every other metadata edit
  instead of being dropped.

Tests

• kale/tests/unit_tests/test_secrets.py (new): validator edge cases,
  StepConfig.secrets defaults/validation, and the steps_defaults
  secret path.
• kale/tests/unit_tests/test_parser.py: tag-parsing success/error
  cases for secret: tags (missing step name, malformed name/key/env,
  multiple secrets on one cell).

Testing

Went through an 8-angle automated code review pass and fixed everything
it surfaced as a real bug before this PR:

• A SecretsDialog row could silently disappear if Secret Name/Key was
  typed before Env Var Name (state now sourced locally, not derived
  fresh from props every render).
• InlineCellsMetadata.tsx never actually passed secrets to the
  mounted editor — the dialog always opened empty.
• secret: tags used as pipeline-wide steps_defaults were accepted
  by validation but silently dropped (no parsing branch existed).
• SECRET_TAG's regex allowed a secret name to start with a digit,
  inconsistent with K8sSecretNameValidator (letter-only), causing a
  confusing two-stage validation mismatch.

Then tested end-to-end against a local KFP cluster (k3d):

1. Tagged a step, added a db-credentials / password → DB_PASSWORD
   mapping via the SECRETS dialog.
2. Compiled with make kfp-compile and confirmed the generated DSL
   calls use_secret_as_env(...) with the right secret_key_to_env.
3. Created the matching db-credentials Kubernetes Secret in the
   cluster's namespace.
4. Ran the pipeline (make kfp-run) and confirmed
   os.environ.get("DB_PASSWORD") inside the step returned the actual
   secret value (verified via the step's output artifact/logs).

Test plan

• make test-backend-unit
• make test-labextension
• Manual UI test: add/edit/remove secret rows in the dialog,
  confirm they persist across dialog close/reopen
• End-to-end run against a local KFP cluster with a real
  Kubernetes Secret

🤖 Generated with Claude Code, commanded/reviewed by @cordeirops

[google-oss-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign jesuino for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ada333]

hi @cordeirops I went quickly through the PR and I think it will have to wait for #894 to be implemented - otherwise we would have like 5 configure buttons for step which seems overwhelming ( #889 also adds a configure button for step).

[cordeirops]

hi @cordeirops I went quickly through the PR and I think it will have to wait for #894 to be implemented - otherwise we would have like 5 configure buttons for step which seems overwhelming ( #889 also adds a configure button for step).

@ada333 thanks for the feedback! I'll take a look at #889 and try to work on that as well.

[cordeirops]

Update: rebased onto #908 (closes #894)

Depends on:

• feat: consolidate per-step config buttons into a single Configure dialog #908

This PR now targets feat/unified-config-panel instead of main.

While this was in review, #908 came out of feedback on this same PR:
having a standalone SECRETS button/dialog alongside Base Image, GPU,
and Caching would mean five separate config buttons once #889 (Report)
also lands. #908 closes #894 by consolidating those into a single
"Configure step" dialog with tabs, so this PR now adds Secrets as a
fourth tab there instead of its own button.

What changed since the last review

• Removed the standalone SecretsDialog.tsx and the SECRETS button in
  CellMetadataEditor.tsx.
• Added a SecretsSection inside StepConfigDialog.tsx (from feat: consolidate per-step config buttons into a single Configure dialog #908),
  wired through the same {label, render} tab array Base Image / GPU /
  Caching already use.
• CellMetadataEditor.tsx now threads secrets/updateSecrets into
  StepConfigDialog instead of rendering its own dialog.
• No change to the backend (tag format, StepConfig.secrets,
  use_secret_as_env compilation) or to TagsUtils/useCellTags -
  those pieces are untouched from the original review.

Why local state instead of deriving from props

Base Image/GPU/Caching in #908 mostly derive their displayed value
directly from props. Secrets can't: an in-progress row (e.g. Secret
Name typed but Env Var Name not yet filled in) has no envName key
yet, so it can't be represented in the committed secrets map without
being excluded and re-appearing incomplete. Local row state is kept,
resynced via a lazy useState initializer that runs once when the
section mounts - which, since this section only mounts when its tab is
selected, already happens exactly when it needs to.

Test plan

• make test-backend-unit, make test-labextension
• Manual test in a running JupyterLab: opened Configure, confirmed
  the Secrets tab renders alongside Base Image/GPU/Caching,
  pre-populates from existing secret: tags, adding/editing rows
  commits correctly, and rows survive switching to another tab and
  back.
• End-to-end run against a local KFP cluster with a real Kubernetes
  Secret (re-verified after the rebase).

🤖 Generated with Claude Code, reviewed by @cordeirops

[cordeirops]

Hi @ada333, since #908 has been closed, could you please take another look at this issue? I have rebased this PR onto the changes from #908.

Please let me know if it needs any changes.

[harshhh817]

Not a maintainer — just a contributor who's been working in nbprocessor.py lately. Nice feature, and I like that the malformed-tag cases (wrong arity, uppercase secret name, env var starting with a digit) are in test_parse_metadata_exc rather than only the happy path.

Three observations, one of which is just a heads-up.

1. The same rules are encoded twice, in two files.

SECRET_TAG and the new validators independently spell out the same constraints:

	in SECRET_TAG	in validators.py
secret name	[a-z]([a-z0-9-]*[a-z0-9])?	K8sNameValidator.regex (identical)
secret key	[-._a-zA-Z0-9]+	K8sSecretKeyValidator.regex (identical)
env var	[a-zA-Z_][a-zA-Z0-9_]*	EnvVarNameValidator.regex (identical)

They agree today, but nothing keeps them in sync — relax one and the other silently keeps rejecting (or accepting). Building SECRET_TAG from the validator patterns, or from shared fragments, would make that impossible.

2. Related: the validators don't run on the pipeline-level path.

secret: is in _STEPS_DEFAULTS_LANGUAGE, so it can also be set as a pipeline-wide default. Those go through _apply_steps_defaults → step.config.update(...), and Config.update assigns via self._set(...) without invoking validators. So for pipeline-level secret tags, SECRET_TAG is the only gate and K8sSecretsValidator never fires.

That's pre-existing behaviour (limits / labels / annotations have the same property), not something this PR introduces — but it does mean the new validators only protect the cell-level path, which makes point 1 matter more than it first looks. Worth knowing when deciding how much to invest in the validators.

3. Dotted Secret names are rejected — intentional?

Kubernetes Secret names are DNS subdomain names, so my.secret is legal. K8sNameValidator implements the DNS label rule (no dots), and SECRET_TAG matches it, so:

secret:my.secret:password:DB_PASSWORD   ->  rejected

Since the tag is :-delimited, a dot wouldn't create any parsing ambiguity, so this looks like an inherited limitation rather than a deliberate choice. Fine to leave, but a user with a dotted Secret name currently can't use the feature at all.

---

Also a heads-up rather than a review point: this adds SECRET_TAG and the secrets key to nbprocessor.py, and #909 moves that whole constants block into kale/processors/constants.py. Whichever merges second will need a small rebase — happy to absorb SECRET_TAG and a SECRETS key on my side if this one lands first.

On the template — the interpolated values end up inside string literals in generated Python, so it's the validators/regex that keep quotes out of secret_name / secret_key. Worth a brief comment there so nobody loosens those patterns later without noticing the coupling.

[ada333]

@cordeirops great work! I have some comments but otherwise looks good.

[ada333]

I would use string constants especially if string is used in multiple places

[ada333]

Same problem is also in TagsUtils.py

[ada333]

this allows to have secret with only env name - I think all the fields (secret name, secret key) are mandatory so I would check existence of all the fields

[ada333]

I would also add validation here in this function - for the regex. It will be better for the user to see that the secret does not fit the regex in UI rather than after compiling in some backend error.

[ada333]

Also I think it would be good to add some restriction of adding new secrets (disabled button f.e.) when the secret before is not in correct format (regex or has empty fields).
Right now there can be many empty secrets:

[ada333]

Not a maintainer — just a contributor who's been working in nbprocessor.py lately. Nice feature, and I like that the malformed-tag cases (wrong arity, uppercase secret name, env var starting with a digit) are in test_parse_metadata_exc rather than only the happy path.

Three observations, one of which is just a heads-up.

1. The same rules are encoded twice, in two files.

SECRET_TAG and the new validators independently spell out the same constraints:

in SECRET_TAG in validators.py
secret name [a-z]([a-z0-9-]*[a-z0-9])? K8sNameValidator.regex (identical)
secret key [-._a-zA-Z0-9]+ K8sSecretKeyValidator.regex (identical)
env var [a-zA-Z_][a-zA-Z0-9_]* EnvVarNameValidator.regex (identical)
They agree today, but nothing keeps them in sync — relax one and the other silently keeps rejecting (or accepting). Building SECRET_TAG from the validator patterns, or from shared fragments, would make that impossible.

2. Related: the validators don't run on the pipeline-level path.

secret: is in _STEPS_DEFAULTS_LANGUAGE, so it can also be set as a pipeline-wide default. Those go through _apply_steps_defaults → step.config.update(...), and Config.update assigns via self._set(...) without invoking validators. So for pipeline-level secret tags, SECRET_TAG is the only gate and K8sSecretsValidator never fires.

That's pre-existing behaviour (limits / labels / annotations have the same property), not something this PR introduces — but it does mean the new validators only protect the cell-level path, which makes point 1 matter more than it first looks. Worth knowing when deciding how much to invest in the validators.

3. Dotted Secret names are rejected — intentional?

Kubernetes Secret names are DNS subdomain names, so my.secret is legal. K8sNameValidator implements the DNS label rule (no dots), and SECRET_TAG matches it, so:

secret:my.secret:password:DB_PASSWORD   ->  rejected

Since the tag is :-delimited, a dot wouldn't create any parsing ambiguity, so this looks like an inherited limitation rather than a deliberate choice. Fine to leave, but a user with a dotted Secret name currently can't use the feature at all.

Also a heads-up rather than a review point: this adds SECRET_TAG and the secrets key to nbprocessor.py, and #909 moves that whole constants block into kale/processors/constants.py. Whichever merges second will need a small rebase — happy to absorb SECRET_TAG and a SECRETS key on my side if this one lands first.

On the template — the interpolated values end up inside string literals in generated Python, so it's the validators/regex that keep quotes out of secret_name / secret_key. Worth a brief comment there so nobody loosens those patterns later without noticing the coupling.

agree especially with the first point

[cordeirops]

@ada333 and @harshhh817, thanks for the review and feedback.
Addressed the review feedback from both of you.

@ada333

1. SECRET_TAG (nbprocessor.py) now derives from the same validators applied to StepConfig.secrets (K8sSecretNameValidator/K8sSecretKeyValidator/EnvVarNameValidator) instead of duplicating the regex inline. Also pulled secret_name/secret_key out into shared constants in validators.py.
2. A Secrets row is now only written to the notebook's tags once every field (secret name, secret key, env var name) is present and matches its expected format, not just when env var name is filled in.
3. Same check now gates the "Add Secret" button too, so you can't add another row until the current one is complete and valid.

@harshhh817

1. Fixed by the same SECRET_TAG change above, since it's now built from the validators instead of a separately hand-written pattern.
2. K8sSecretNameValidator now validates Secret names as DNS subdomains (dots allowed), matching actual Kubernetes Secret name rules, instead of inheriting the stricter single-label rule used for other resource names. SECRET_TAG picks this up automatically since it's derived from the validator.
3. Added a comment in the template noting that the validators are what keep quote characters out of the interpolated values, since they land inside string literals in generated code.

The pipeline-level steps_defaults path bypassing field validators and the kale/processors/constants.py move in #909 are both out of scope here, as you noted.

Also fixed a UX bug found during testing: an in-progress Secrets row (not yet valid, so not written to tags) was silently lost when you switched to another tab and back, because the Secrets tab's content unmounts on tab switch. It now survives tab switches and only clears when the dialog itself closes.

All changes pushed to this branch. Let me know if anything's still off.

[harshhh817]

Deriving SECRET_TAG from the validators is exactly the right shape — the two genuinely can't drift now, and _regex_body keeps it readable.

I re-composed the new patterns and checked the cases I had in mind:

tag	matches
secret:db-credentials:password:DB_PASSWORD	✅
secret:my.secret:password:DB_PASSWORD	✅ (was rejected before)
secret:team.db-creds:tls.key:TLS_KEY	✅
secret:DB-creds:...	❌
secret:db-creds:password:1_INVALID	❌
secret:db..creds:password:VAR	❌

So dotted Secret names work and the empty-label case is still rejected. Agreed the steps_defaults validator bypass is out of scope here — it's pre-existing and affects limits/labels equally, probably worth its own issue rather than riding along on this.

Nice catch on the tab-switch state loss too. LGTM from my side (non-binding — not a maintainer).

[ada333]

@cordeirops thanks for the changes, I noticed when adding secrets to pipeline steps it is not in the metadata text above the cell, unlike other step properties (GPU request, Base Image, ...) :

[cordeirops]

@harshhh817 @ada333 thanks for the extra look.

@harshhh817, appreciate you re-checking the SECRET_TAG/validator patterns and confirming dotted names now work, and the empty-label case is still rejected. Agreed, the steps_defaults validator bypass deserves its own issue rather than riding along here.

@ada333 good catch, Secrets weren't showing up in the metadata text above the cell like GPU request, Base Image, Caching, and HTML Report do. Fixed: the inline text now includes a "Secrets: ENV_NAME, ..." line listing the configured env var names, same pattern as the other properties.

Pushed in 379e134.

[ada333]

LGTM