Skip to content

Encapsulate OPENFAAS_NS lookup - #1029

Open
welteki wants to merge 1 commit into
openfaas:masterfrom
welteki:refactor/self-contained-namespace-env
Open

Encapsulate OPENFAAS_NS lookup#1029
welteki wants to merge 1 commit into
openfaas:masterfrom
welteki:refactor/self-contained-namespace-env

Conversation

@welteki

@welteki welteki commented Sep 7, 2026

Copy link
Copy Markdown
Member

Description

Move the OPENFAAS_NS environment lookup into getNamespace and restore its two-argument signature.

Update namespace-aware commands and tests to use the self-contained helper. Namespace precedence and behaviour remain unchanged.

Motivation and Context

This is a follow-up to #1028 and its review discussion.

Every caller previously passed the same os.Getenv("OPENFAAS_NS") value into getNamespace. Keeping the lookup within the helper removes repetitive plumbing and keeps namespace resolution in one place.

  • I have raised an issue to propose this change (required)

How Has This Been Tested?

The namespace resolution unit tests set OPENFAAS_NS through the test environment and cover the environment fallback along with flag and stack namespace precedence.

The CLI was also tested against an OpenFaaS gateway with OPENFAAS_NS set, verifying that it resolved the namespace and listed functions successfully.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have signed-off my commits with git commit -s
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Read OPENFAAS_NS inside getNamespace so callers no longer pass the same
environment value explicitly.

Signed-off-by: Han Verstraete (OpenFaaS Ltd) <han@openfaas.com>
@reviewfn

reviewfn Bot commented Sep 7, 2026

Copy link
Copy Markdown

AI Pull Request Overview

Disclaimer: This review was generated by automated AI and may contain errors. Do not trust its outputs without human verification.

Summary

  • Centralizes OPENFAAS_NS lookup inside getNamespace.
  • Updates namespace-aware command call sites to use the restored two-argument helper.
  • Keeps namespace precedence as flag, stack namespace, environment, then default.
  • Updates namespace precedence tests to set OPENFAAS_NS through the process environment.
  • No concrete behavioral regression found in the scoped diff.

Approval rating (1-10)

9/10. The change is focused, preserves existing precedence, and updates relevant tests; no blocking issue was identified.

Summary per file

Summary per file
File path Summary
commands/deploy.go Uses centralized namespace helper in deploy paths.
commands/describe.go Uses centralized namespace helper for function lookup.
commands/diff.go Updates diff namespace resolution to the new helper signature.
commands/diff_test.go Sets OPENFAAS_NS via test environment for diff precedence coverage.
commands/invoke.go Uses centralized namespace helper for invocation namespace resolution.
commands/list.go Uses centralized namespace helper for list requests.
commands/logs.go Uses centralized namespace helper for log requests.
commands/priority.go Moves OPENFAAS_NS lookup into getNamespace.
commands/priority_test.go Updates namespace tests to exercise environment-backed lookup.
commands/ready.go Uses centralized namespace helper for readiness checks.
commands/remove.go Uses centralized namespace helper in stack and direct delete paths.
commands/secret_apply.go Uses centralized namespace helper for secret apply.
commands/secret_create.go Uses centralized namespace helper for secret creation.
commands/secret_list.go Uses centralized namespace helper for secret listing.
commands/secret_remove.go Uses centralized namespace helper for secret removal.
commands/secret_update.go Uses centralized namespace helper for secret updates.
commands/store_deploy.go Uses centralized namespace helper for store deploy.

Overall Assessment

The PR is a small refactor with the intended behavior preserved in the final state. getNamespace still applies the same precedence order, and the changed command call sites consistently use the restored two-argument signature. The updated tests cover environment fallback and precedence through the actual OPENFAAS_NS environment variable rather than a direct helper parameter, which matches the new encapsulation.

Detailed Review

Detailed Review

No blocking findings.

The scoped changes are internally consistent: all reviewed getNamespace call sites now use the two-argument signature, and namespacesForDiff delegates to the helper so diff behavior continues to share the same namespace precedence as the other commands.

The only meaningful behavior shift is that getNamespace reads OPENFAAS_NS at call time instead of receiving a value captured by each caller. In the reviewed command paths this should be equivalent because the environment is not mutated during command execution.

The updated tests are aligned with the new design because they validate environment fallback through t.Setenv(openFaaSNamespaceEnvironment, ...), including the stack namespace precedence case after stack-file substitution.

AI agent details.

Agent processing time: 57.779s
Environment preparation time: 3.752s
Total time from webhook: 1m7.301s

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant