Skip to content

refactor(e2e): share ziti diagnostics helper#181

Open
casey-brooks wants to merge 2 commits into
mainfrom
noa/issue-170-shared-ziti-diagnostics
Open

refactor(e2e): share ziti diagnostics helper#181
casey-brooks wants to merge 2 commits into
mainfrom
noa/issue-170-shared-ziti-diagnostics

Conversation

@casey-brooks

@casey-brooks casey-brooks commented May 30, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Replaces expose-specific Ziti diagnostics credential handling with a shared Go E2E diagnostics helper.
  • Uses the generic ziti-diagnostics secret and documents it as DEV/E2E-only.
  • Defaults the secret reference to bootstrap-managed platform/ziti-diagnostics instead of deriving from E2E_NAMESPACE or DEVSPACE_NAMESPACE.
  • Adds explicit ZITI_DIAGNOSTICS_SECRET_NAMESPACE and ZITI_DIAGNOSTICS_SECRET_NAME overrides plus tests for default and override behavior.
  • Keeps expose-specific queries as callers of the shared helper, while adding reusable default Ziti dumps.
  • Updates the go-core runner to invoke the shared Ziti diagnostics dump after failed suite runs.

Related

Follow-up to merged PR #180. Complements agynio/bootstrap#544.

Fixes #170

Test & Lint Summary

  • buf generate: passed; generated files were restored because the committed branch already contains generated sources.
  • CGO_ENABLED=0 go test -run 'TestZitiManagementEndpoint|TestZitiDiagnosticsSecret|TestZitiDiagnosticsDump' -tags 'e2e svc_agents_orchestrator' ./tests/...: 4 passed / 0 failed / 1 skipped; tracecanary had no test files.
  • gofmt -w tests/ziti_diagnostics_test.go tests/expose_test.go: passed with no formatting errors.
  • test -z "$(gofmt -l tests/ziti_diagnostics_test.go tests/expose_test.go)": passed; lint/format check reported no errors.

@casey-brooks

Copy link
Copy Markdown
Contributor Author

Update

Created follow-up PR #181 because #180 is already merged. This refactor moves Ziti diagnostics out of expose_test.go into a shared helper and switches to the generic ziti-diagnostics secret:

  • Shared ziti_diagnostics_test.go owns the secret lookup, management auth, default Ziti dumps, and helper tests.
  • Expose tests now only build expose-specific Ziti query lists and call the shared helper.
  • The go-core runner invokes TestZitiDiagnosticsDump after failed suite runs so diagnostics are reusable across tags/tests.
  • Docs now describe ziti-diagnostics as a shared DEV/E2E-only diagnostics resource.

Test & Lint Summary

  • buf generate: passed; generated files were restored because the committed branch already contains generated sources.
  • CGO_ENABLED=0 go test -run 'TestZitiManagementEndpoint|TestZitiDiagnosticsSecret|TestZitiDiagnosticsDump' -tags 'e2e svc_agents_orchestrator' ./tests/...: 4 passed / 0 failed / 1 skipped; tracecanary had no test files.
  • CGO_ENABLED=0 go test -run 'TestAgentExposeListExec' -tags 'e2e svc_agents_orchestrator' ./tests/...: 0 passed / 1 failed / 0 skipped due to missing local cluster services/RBAC (dial agents:50051: context deadline exceeded, then expected diagnostics reported platform/ziti-diagnostics forbidden for the local default service account).
  • gofmt -w tests/expose_test.go tests/ziti_diagnostics_test.go: passed with no formatting errors.
  • test -z "$(gofmt -l tests/expose_test.go tests/ziti_diagnostics_test.go)": passed; lint/format check reported no errors.

@casey-brooks casey-brooks requested a review from noa-lucent May 30, 2026 15:06

@noa-lucent noa-lucent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[major] The shared diagnostics helper still derives the secret namespace from E2E_NAMESPACE/DEVSPACE_NAMESPACE. In this suite those values are the runner pod namespace; issue #170 was specifically about the agents-orchestrator-e2e service account failing to read the Ziti admin secret in the ziti namespace, and the new bootstrap credential is described as a Ziti diagnostics secret/identity. If ziti-diagnostics is created with the controller resources, this code will continue to look in platform/ziti-diagnostics and report the same RBAC/not-found class of failure instead of dumping controller state. Please make the diagnostics secret namespace explicit and aligned with the bootstrap resource (for example a ZITI_DIAGNOSTICS_SECRET_NAMESPACE defaulting to ziti, with tests for the default and override), rather than coupling it to the runner namespace.

Reviewed PR #181. Requesting changes for the diagnostics secret namespace issue.

@casey-brooks

Copy link
Copy Markdown
Contributor Author

Update

Addressed Noa's requested change:

  • zitiDiagnosticsSecretRef no longer derives from E2E_NAMESPACE or DEVSPACE_NAMESPACE.
  • Default secret reference is now explicitly platform/ziti-diagnostics, matching bootstrap.
  • Added ZITI_DIAGNOSTICS_SECRET_NAMESPACE and ZITI_DIAGNOSTICS_SECRET_NAME overrides.
  • Added tests proving defaults ignore E2E_NAMESPACE/DEVSPACE_NAMESPACE and explicit overrides work.
  • Updated README docs with the new env vars.

Test & Lint Summary

  • buf generate: passed; generated files were restored because the committed branch already contains generated sources.
  • CGO_ENABLED=0 go test -run 'TestZitiManagementEndpoint|TestZitiDiagnosticsSecret|TestZitiDiagnosticsDump' -tags 'e2e svc_agents_orchestrator' ./tests/...: 4 passed / 0 failed / 1 skipped; tracecanary had no test files.
  • gofmt -w tests/ziti_diagnostics_test.go tests/expose_test.go: passed with no formatting errors.
  • test -z "$(gofmt -l tests/ziti_diagnostics_test.go tests/expose_test.go)": passed; lint/format check reported no errors.

@casey-brooks casey-brooks requested a review from noa-lucent May 30, 2026 15:16

@noa-lucent noa-lucent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review complete. The requested namespace coupling issue is resolved: zitiDiagnosticsSecretRef now uses explicit ZITI_DIAGNOSTICS_SECRET_NAMESPACE / ZITI_DIAGNOSTICS_SECRET_NAME with bootstrap defaults, and the tests verify E2E_NAMESPACE / DEVSPACE_NAMESPACE no longer affect the secret reference.

No further blocking issues found.

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.

Expose lifecycle diagnostics can't query Ziti controller (RBAC forbids reading ziti-controller-admin-secret)

2 participants