Skip to content

Conversation

@EronWright
Copy link
Contributor

@EronWright EronWright commented Oct 18, 2025

Summary

Fixes #935

The secretsProvider field from Stack CRs was being ignored when the agent created new stacks, causing Pulumi to default to passphrase encryption and fail when users specified alternative providers like HashiVault or AWS KMS.

Not in Scope:

Changes

This PR addresses the issue identified by @brycekahle where the secretsProvider was being sent in the SelectStackRequest from the workspace controller but was not being used by the agent when creating new stacks.

Key Changes:

  1. Created initStack helper function - Uses pulumi stack init --secrets-provider to properly initialize stacks with the specified secrets provider from the beginning, instead of the two-step approach of creating the stack first and then changing the secrets provider.

  2. Fixed SelectStack handler - Now properly applies the secrets_provider field from the SelectStackRequest when creating new stacks by calling initStack.

  3. Fixed NewServer - Changed from auto.UpsertStack + ChangeSecretsProvider to auto.SelectStack with fallback to initStack for consistent behavior across both code paths.

  4. Added --secrets-provider flag to the serve command for standalone usage.

Testing

The integration tests verify that the --secrets-provider argument is applied correctly to pulumi stack init.

  • Added test case in TestNewServer for creating new stacks with invalid secrets provider
  • Added test case in TestSelectStack for creating new stacks with invalid secrets provider
  • Both tests verify that appropriate error messages are returned when an invalid provider is specified

Files Changed

  • agent/cmd/serve.go - Added --secrets-provider flag and passed it to NewServer
  • agent/pkg/server/server.go - Implemented initStack helper, refactored stack creation to properly handle the secrets_provider field in both SelectStack and NewServer, added debug logging
  • agent/pkg/server/server_test.go - Added test coverage for invalid secrets provider scenarios

🤖 Generated with Claude Code

Co-Authored-By: Claude [email protected]

@EronWright EronWright marked this pull request as draft October 18, 2025 05:00
@codecov
Copy link

codecov bot commented Oct 18, 2025

Codecov Report

❌ Patch coverage is 60.78431% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 53.28%. Comparing base (f2a3513) to head (68f121c).
⚠️ Report is 42 commits behind head on master.

Files with missing lines Patch % Lines
agent/pkg/server/server.go 63.04% 14 Missing and 3 partials ⚠️
agent/cmd/serve.go 40.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1032      +/-   ##
==========================================
+ Coverage   53.03%   53.28%   +0.24%     
==========================================
  Files          34       34              
  Lines        4646     5011     +365     
==========================================
+ Hits         2464     2670     +206     
- Misses       1987     2134     +147     
- Partials      195      207      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@EronWright EronWright requested a review from Copilot October 18, 2025 06:34
Copilot

This comment was marked as outdated.

EronWright and others added 6 commits October 18, 2025 11:48
The secretsProvider field from Stack CRs was being ignored when the
agent created new stacks via SelectStack, causing Pulumi to default
to passphrase encryption and fail when users specified alternative
providers like HashiVault or AWS KMS.

Changes:
- Apply secrets provider in SelectStack when creating new stacks
- Apply secrets provider in NewServer only for new stacks (not existing)
- Add --secrets-provider flag to serve command for standalone usage
- Clarify that secrets provider only applies to new stack creation

Fixes #935

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
This commit improves the implementation and adds unit test coverage for
the secretsProvider field that was added in the fix for issue #935.

Changes:
- Refactor SelectStack to use `pulumi stack init --secrets-provider`
  instead of auto.NewStack + ChangeSecretsProvider. This properly
  initializes the secrets provider from the beginning and avoids
  requiring PULUMI_CONFIG_PASSPHRASE_NEW environment variable.

- Add test case "non-existent stack with create and passphrase secrets
  provider" to TestSelectStack that validates the secretsProvider field
  is properly applied when creating new stacks.

- Remove t.Parallel() from TestSelectStack to allow t.Setenv() usage
  for setting test-specific environment variables.

The test validates that when a SelectStackRequest includes both
create=true and a secretsProvider value, the stack is created with
the specified secrets provider configuration.

Related to #935

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
…in NewServer

Updated NewServer to use the same stack creation technique as SelectStack
by calling `pulumi stack init --secrets-provider` instead of the two-step
approach of auto.NewStack followed by ChangeSecretsProvider. This ensures
the secrets provider is properly initialized from the beginning and provides
consistency across both code paths.

Also added test coverage in TestNewServer for the secrets provider functionality,
following the same pattern used in TestSelectStack (removed t.Parallel and added
setPassenv field).

Related to #935

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
- Renamed newStackWithOptions to initStack for better clarity
- Added structured debug logging throughout the function to trace:
  - Stack initialization start with parameters
  - Pulumi command execution
  - Stack creation success/failure
  - Stack selection after creation
- Moved function definition after SelectStack for better code organization

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
- Capture stderr from pulumi stack init command and include it in error messages
- Replace passphrase secrets provider tests with invalid provider tests
- Invalid provider tests verify that CLI error messages are properly surfaced
- Add t.Parallel() to TestNewServer and TestSelectStack for faster test execution
- Remove passphrase-specific validation logic from tests

This change makes the tests more effective by:
1. Testing actual error conditions that users might encounter
2. Being backend-agnostic (works regardless of default secrets provider)
3. Validating that stderr is properly captured and surfaced in errors

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@EronWright EronWright marked this pull request as ready for review October 18, 2025 19:10
@EronWright EronWright requested review from a team and blampe October 18, 2025 19:10
@EronWright EronWright requested a review from Copilot October 18, 2025 19:18
@EronWright EronWright self-assigned this Oct 18, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@EronWright EronWright enabled auto-merge (squash) October 18, 2025 19:19
@EronWright EronWright merged commit 2a6441f into master Oct 20, 2025
9 checks passed
@EronWright EronWright deleted the issue-935 branch October 20, 2025 16:05
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.

secretsProvider is not passed from stack crd to stack yaml config

2 participants