Skip to content

Require Ziti cleanup on app deletion#11

Merged
rowan-stein merged 2 commits into
mainfrom
noa/issue-10
Apr 19, 2026
Merged

Require Ziti cleanup on app deletion#11
rowan-stein merged 2 commits into
mainfrom
noa/issue-10

Conversation

@casey-brooks
Copy link
Copy Markdown
Contributor

Summary

  • require successful Ziti identity cleanup before deleting apps
  • keep best-effort cleanup for rollback paths
  • add deletion tests and update fakes for new gRPC methods

Testing

  • buf generate --template buf.gen.yaml
  • go vet ./...
  • go test ./...

Refs #10

@casey-brooks
Copy link
Copy Markdown
Contributor Author

Summary

  • require successful Ziti identity cleanup before deleting apps
  • keep rollback cleanup best-effort
  • add deletion tests and update fake gRPC clients

Test & Lint

  • buf generate --template buf.gen.yaml
  • go vet ./...
  • go test ./...

Tests: passed 2, failed 0, skipped 0
Lint: no issues

Copy link
Copy Markdown

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

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

Requesting changes.

  • [major] Make DeleteApp retryable/idempotent when Ziti cleanup was already performed (e.g., handle gRPC NotFound from DeleteAppIdentity as success) and add test coverage.
  • [minor] Strengthen TestDeleteAppFailsWhenZitiCleanupFails to assert we don’t attempt authz cleanup when we abort.

Comment thread internal/server/server.go
Comment thread internal/server/server_test.go
@casey-brooks
Copy link
Copy Markdown
Contributor Author

Summary

  • treat Ziti DeleteAppIdentity NotFound as success for idempotent deletes
  • add NotFound retry coverage and strengthen cleanup-failure test expectations

Test & Lint

  • buf generate --template buf.gen.yaml
  • go vet ./...
  • go test ./...

Tests: passed 2, failed 0, skipped 0
Lint: no issues

Copy link
Copy Markdown

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

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

Re-review complete. Requested changes addressed:\n- DeleteApp is now idempotent/retryable when Ziti cleanup returns NotFound.\n- Tests cover NotFound path and assert no authz/store cleanup occurs when Ziti cleanup fails.\n\nApproved.

@rowan-stein rowan-stein merged commit 312822c into main Apr 19, 2026
1 check passed
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.

3 participants