Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use a per-stack PULUMI_HOME directory #490

Merged
merged 12 commits into from
Sep 22, 2023
Merged

Conversation

EronWright
Copy link
Contributor

@EronWright EronWright commented Sep 9, 2023

Proposed changes

Closes #483

This PR seeks to isolate the credentials associated with a given Stack, to solve the problem of credentials leaking across stacks. Some underlying details here:

  1. Pulumi CLI stores login credentials in PULUMI_HOME (e.g. ~/.pulumi/credentials.json).
  2. A side-effect of using PULUMI_ACCESS_TOKEN is that the CLI login credentials are set.
  3. Pulumi CLI prefers the persisted login credentials to PULUMI_ACCESS_TOKEN.

This PR takes the conservative approach of encapsulating the PULUMI_HOME into a per-stack working directory, as opposed to reusing ~/.pulumi across stacks. The working directory is retained across reconciliation passes, and cleaned up during stack finalization. Note that the workspace directory is erased at the end of each reconciliation pass, as is the current behavior.

This PR does NOT solve the (lack of) mutability of PULUMI_ACCESS_TOKEN across stack updates.

Note that this PR contains some commits (related to hacking on the operator) that will be moved to a separate PR.

Technical Details

Relevant terminology used within the controller codebase:

  • root directory - a temporary directory for each stack, retained until finalization
  • home directory - the PULUMI_HOME directory, located within the stack's root directory
  • workspace directory - the Pulumi workspace directory containing the program and stack configuration.

The current behavior of the operator is to erase the workspace directory on each reconciliation pass, e.g. to ensure a clean git checkout. This PR retains this behavior while keeping the home directory across passes, e.g. to reuse the providers.

Related issues (optional)

@EronWright EronWright changed the title Eronwright/issue 483 Use a per-stack PULUMI_HOME directory Sep 9, 2023
@EronWright EronWright marked this pull request as ready for review September 19, 2023 00:02
@EronWright EronWright requested a review from a team September 19, 2023 00:17
pkg/controller/stack/stack_controller.go Outdated Show resolved Hide resolved
pkg/controller/stack/stack_controller.go Outdated Show resolved Hide resolved
pkg/controller/stack/stack_controller.go Show resolved Hide resolved
pkg/controller/stack/stack_controller.go Outdated Show resolved Hide resolved
pkg/controller/stack/flux.go Outdated Show resolved Hide resolved
pkg/controller/stack/stack_controller.go Outdated Show resolved Hide resolved
pkg/controller/stack/stack_controller.go Outdated Show resolved Hide resolved
pkg/controller/stack/stack_controller.go Outdated Show resolved Hide resolved
pkg/controller/stack/stack_controller.go Outdated Show resolved Hide resolved
test/stack_isolation_test.go Show resolved Hide resolved
test/stack_isolation_test.go Show resolved Hide resolved
It("should purge the workspace dir", func() {
rootDir := getRootDir(stack)
Expect(k8sClient.Create(context.TODO(), stack)).To(Succeed())
waitForStackSuccess(stack)
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to also somehow include a check that the workspace folder existing when the stack is being reconciled to validate that we are indeed using a workspace folder.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry I don't see a decent way to do that.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking of just spawning a goroutine that polls the existence of the workspace folder. Something like:

It("should purge the workspace dir", func() {
	rootDir := getRootDir(stack)
	Expect(k8sClient.Create(context.TODO(), stack)).To(Succeed())
	// Spy on the root directory in separate goroutine to make sure the workspace dir is created.
	var workspaceCreated bool
	done := make(chan struct{})
	go func() {
		for {
			select {
			case <-time.After(2 * time.Second):
				if exists(filepath.Join(rootDir, "workspace")) {
					workspaceCreated = true
					return
				}
			case <-done:
				return
			}
		}
	}()
	waitForStackSuccess(stack)
	close(done) // Clean-up spawned goroutine.
	Expect(workspaceCreated).To(BeTrue())
	Expect(exists(filepath.Join(rootDir, "workspace"))).To(BeFalse())
})

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not thrilled at the lack of determinism, I think I'll try to make a hook for testing purposes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's expand on this in a follow-up PR.

@EronWright EronWright merged commit 4d88f98 into master Sep 22, 2023
5 checks passed
@EronWright EronWright deleted the eronwright/issue-483 branch September 22, 2023 15:58
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.

Use different ACCESS_TOKENS for different Stacks
3 participants