From 7c9766d18e5cbcc407133fb45727bd0b2c0fa3e7 Mon Sep 17 00:00:00 2001 From: Moritz Clasmeier Date: Mon, 27 Apr 2026 11:27:09 +0200 Subject: [PATCH 1/9] Create a temp directory for Deployer and add a Cleanup() method --- cmd/deploy.go | 1 + internal/deployer/deployer.go | 27 +++++++++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/cmd/deploy.go b/cmd/deploy.go index 3343c330..f3ea8695 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -124,6 +124,7 @@ func runDeploy(cmd *cobra.Command, args []string) error { if err != nil { return fmt.Errorf("failed to create deployer: %w", err) } + defer d.Cleanup() if overrideFile != "" { var err error diff --git a/internal/deployer/deployer.go b/internal/deployer/deployer.go index 2d50d3d2..007b3e2a 100644 --- a/internal/deployer/deployer.go +++ b/internal/deployer/deployer.go @@ -133,6 +133,7 @@ type Deployer struct { securedClusterWaitTimeout time.Duration dockerCreds *dockerauth.Credentials clusterResourceKinds map[string]struct{} + tempDir string } type ResourceToDelete struct { @@ -502,6 +503,10 @@ func (d *Deployer) SetFeatureFlags(featureFlags []string) error { return nil } +// New creates a new Deployer instance. +// It verifies that the current environment contains necessary tools. +// It creates a temporary directory for the deployer to use during deployment, +// and it is the caller's responsibility to clean it up using the Cleanup() method when not used anymore. func New(log *logger.Logger) (*Deployer, error) { if err := checkRequiredTools(); err != nil { return nil, err @@ -512,6 +517,11 @@ func New(log *logger.Logger) (*Deployer, error) { return nil, err } + tempDir, err := os.MkdirTemp("", "roxie-deployer-*") + if err != nil { + return nil, fmt.Errorf("failed to create temporary directory: %w", err) + } + d := &Deployer{ logger: log, startTime: time.Now(), @@ -522,6 +532,7 @@ func New(log *logger.Logger) (*Deployer, error) { shouldDeployOperator: true, centralWaitTimeout: DefaultCentralWaitTimeout, securedClusterWaitTimeout: DefaultSecuredClusterWaitTimeout, + tempDir: tempDir, } d.dockerAuth = dockerauth.New(log) @@ -578,6 +589,22 @@ func (d *Deployer) getClusterResourceKinds() (map[string]struct{}, error) { return kinds, nil } +// Cleanup cleans up any temporary resources created by the deployer, such as temporary files. +func (d *Deployer) Cleanup() error { + if d.tempDir != "" && d.envrcFile == "" { + // In the case of envrc file usage, we need to keep temporary files around after deployment. + // (It contains CA certificates, for example.) + if err := os.RemoveAll(d.tempDir); err != nil { + return fmt.Errorf("failed to clean up temporary directory: %w", err) + } + } + return nil +} + +// Deploy deploys the specified components to the cluster. +// +// It might create temporary files in the deployer's tempDir, but it is the caller's responsibility to clean them up +// when not used anymore. func (d *Deployer) Deploy(ctx context.Context, components component.Component, resources, exposure string) error { adjustedResources, adjustedExposure, adjustedPortForward := d.clusterDefaults.ApplyConvenienceDefaults( d.kubeContext, From cb17ae35550fd2d975af2bb45aeb8c6777166c57 Mon Sep 17 00:00:00 2001 From: Moritz Clasmeier Date: Mon, 27 Apr 2026 11:32:00 +0200 Subject: [PATCH 2/9] Don't use hard-coded file path for CA cert --- internal/deployer/deploy_via_operator.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/internal/deployer/deploy_via_operator.go b/internal/deployer/deploy_via_operator.go index 696a35c5..0e708be9 100644 --- a/internal/deployer/deploy_via_operator.go +++ b/internal/deployer/deploy_via_operator.go @@ -5,6 +5,7 @@ import ( "context" "errors" "fmt" + "os" "os/exec" "strings" "time" @@ -570,11 +571,15 @@ func (d *Deployer) fetchCentralCACert(ctx context.Context) error { } // TODO(#91): possible symlink race vulnerability - d.roxCACertFile = "/tmp/roxie-ca-cert.pem" - // TODO(#91): this is a REALLY weird way to write a temporary file in go - writeCmd := exec.CommandContext(ctx, "sh", "-c", fmt.Sprintf("cat > %s", d.roxCACertFile)) - writeCmd.Stdin = bytes.NewReader(caCert) - if err := writeCmd.Run(); err != nil { + caCertFile, err := os.CreateTemp(d.tempDir, "roxie-central-ca-*.pem") + if err != nil { + return fmt.Errorf("failed to create temp file for CA cert: %w", err) + } + + d.roxCACertFile = caCertFile.Name() + if _, err := caCertFile.Write(caCert); err != nil { + caCertFile.Close() + os.Remove(d.roxCACertFile) return fmt.Errorf("failed to write CA cert: %w", err) } From 4fb26b0e15ec30d24941aeb0c9827b6fbbfdcd60 Mon Sep 17 00:00:00 2001 From: Moritz Clasmeier Date: Mon, 27 Apr 2026 12:01:48 +0200 Subject: [PATCH 3/9] Use temp directory in deployer --- internal/deployer/operator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/deployer/operator.go b/internal/deployer/operator.go index 1e51c068..35b990a9 100644 --- a/internal/deployer/operator.go +++ b/internal/deployer/operator.go @@ -73,7 +73,7 @@ func (d *Deployer) deployOperatorNonOLM(ctx context.Context) error { // downloadAndExtractOperatorBundle downloads and extracts the operator bundle func (d *Deployer) downloadAndExtractOperatorBundle(ctx context.Context, bundleImage string) (string, error) { - bundleDir, err := os.MkdirTemp("", "stackrox-operator-bundle-") + bundleDir, err := os.MkdirTemp(d.tempDir, "stackrox-operator-bundle-") if err != nil { return "", fmt.Errorf("failed to create temp dir: %w", err) } From 6d3032f7fb67f35f6aa8e96134582bf431b30463 Mon Sep 17 00:00:00 2001 From: Moritz Clasmeier Date: Mon, 27 Apr 2026 12:28:30 +0200 Subject: [PATCH 4/9] Fix unrelated bug: the environment variables may not contain quotation characters, they would end up as literals in the variable values --- cmd/subshell.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/cmd/subshell.go b/cmd/subshell.go index ec0636ee..4cbb898a 100644 --- a/cmd/subshell.go +++ b/cmd/subshell.go @@ -32,22 +32,22 @@ func spawnSubshell(d *deployer.Deployer, log *logger.Logger) error { endpoint, password, caCertFile, kubeContext, exposure := d.GetDeploymentInfo() if endpoint != "" { - env = append(env, fmt.Sprintf("API_ENDPOINT=%q", endpoint)) - env = append(env, fmt.Sprintf("ROX_ENDPOINT=%q", endpoint)) - env = append(env, fmt.Sprintf("ROX_BASE_URL='https://%s'", endpoint)) + env = append(env, fmt.Sprintf("API_ENDPOINT=%s", endpoint)) + env = append(env, fmt.Sprintf("ROX_ENDPOINT=%s", endpoint)) + env = append(env, fmt.Sprintf("ROX_BASE_URL=https://%s", endpoint)) } if password != "" { - env = append(env, fmt.Sprintf("ROX_ADMIN_PASSWORD=%q", password)) + env = append(env, fmt.Sprintf("ROX_ADMIN_PASSWORD=%s", password)) } if caCertFile != "" { - env = append(env, fmt.Sprintf("ROX_CA_CERT_FILE=%q", caCertFile)) + env = append(env, fmt.Sprintf("ROX_CA_CERT_FILE=%s", caCertFile)) } - env = append(env, fmt.Sprintf("ROX_USERNAME=%q", deployer.AdminUsername)) + env = append(env, fmt.Sprintf("ROX_USERNAME=%s", deployer.AdminUsername)) env = append(env, "ROXIE_SHELL=1") - env = append(env, fmt.Sprintf("name='acs@%s'", kubeContext)) + env = append(env, fmt.Sprintf("name=acs@%s", kubeContext)) haproxyAvailable := isHAProxyAvailable() @@ -61,7 +61,7 @@ func spawnSubshell(d *deployer.Deployer, log *logger.Logger) error { if err != nil { log.Warningf("Failed to start HAProxy: %v", err) } else { - env = append(env, fmt.Sprintf("ROXIE_HAPROXY_CFG_FILE=%q", haproxyConfigPath)) + env = append(env, fmt.Sprintf("ROXIE_HAPROXY_CFG_FILE=%s", haproxyConfigPath)) haproxyStarted = true defer cleanupHAProxy(haproxyCmd, haproxyConfigPath) } From 30f9595402055bd811f19babab4f075e07782966 Mon Sep 17 00:00:00 2001 From: Moritz Clasmeier Date: Mon, 27 Apr 2026 16:46:08 +0200 Subject: [PATCH 5/9] Don't forget to close caCertFile on success --- internal/deployer/deploy_via_operator.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/internal/deployer/deploy_via_operator.go b/internal/deployer/deploy_via_operator.go index 0e708be9..32918fe7 100644 --- a/internal/deployer/deploy_via_operator.go +++ b/internal/deployer/deploy_via_operator.go @@ -578,10 +578,13 @@ func (d *Deployer) fetchCentralCACert(ctx context.Context) error { d.roxCACertFile = caCertFile.Name() if _, err := caCertFile.Write(caCert); err != nil { - caCertFile.Close() - os.Remove(d.roxCACertFile) + _ = caCertFile.Close() + _ = os.Remove(d.roxCACertFile) return fmt.Errorf("failed to write CA cert: %w", err) } + if err := caCertFile.Close(); err != nil { + return fmt.Errorf("failed to close CA cert file: %w", err) + } d.logger.Successf("✓ CA certificate saved to: %s", d.roxCACertFile) return nil From c4218d9e8b9659d51881e9191291c6b2c24653f5 Mon Sep 17 00:00:00 2001 From: Moritz Clasmeier Date: Mon, 27 Apr 2026 16:47:40 +0200 Subject: [PATCH 6/9] Removed TODO --- internal/deployer/deploy_via_operator.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/deployer/deploy_via_operator.go b/internal/deployer/deploy_via_operator.go index 32918fe7..1b9b363d 100644 --- a/internal/deployer/deploy_via_operator.go +++ b/internal/deployer/deploy_via_operator.go @@ -570,7 +570,6 @@ func (d *Deployer) fetchCentralCACert(ctx context.Context) error { return fmt.Errorf("failed to decode CA cert: %w", err) } - // TODO(#91): possible symlink race vulnerability caCertFile, err := os.CreateTemp(d.tempDir, "roxie-central-ca-*.pem") if err != nil { return fmt.Errorf("failed to create temp file for CA cert: %w", err) From d7be7d297bb7a1bd44098a033dc9edcd5bbf9a21 Mon Sep 17 00:00:00 2001 From: Moritz Clasmeier Date: Mon, 27 Apr 2026 16:50:38 +0200 Subject: [PATCH 7/9] Log on cleanup error --- cmd/deploy.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cmd/deploy.go b/cmd/deploy.go index f3ea8695..6da13b9c 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -124,7 +124,11 @@ func runDeploy(cmd *cobra.Command, args []string) error { if err != nil { return fmt.Errorf("failed to create deployer: %w", err) } - defer d.Cleanup() + defer func() { + if err := d.Cleanup(); err != nil { + log.Warningf("Failed to invoke deployer cleanup: %v", err) + } + }() if overrideFile != "" { var err error From f3f0d99b6f1ed11de361b5072802ea5e76595a33 Mon Sep 17 00:00:00 2001 From: Moritz Clasmeier Date: Mon, 27 Apr 2026 17:58:49 +0200 Subject: [PATCH 8/9] Cleanup after teardown --- cmd/teardown.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cmd/teardown.go b/cmd/teardown.go index 015d8cdf..c96b3ea8 100644 --- a/cmd/teardown.go +++ b/cmd/teardown.go @@ -44,6 +44,11 @@ func runTeardown(cmd *cobra.Command, args []string) error { if err != nil { return fmt.Errorf("failed to create deployer: %w", err) } + defer func() { + if err := d.Cleanup(); err != nil { + log.Warningf("Failed to invoke deployer cleanup: %v", err) + } + }() d.SetSingleNamespace(singleNamespace) From 5171bcf42d51227e7af46f0c4bfb97f9323ed367 Mon Sep 17 00:00:00 2001 From: Moritz Clasmeier Date: Tue, 28 Apr 2026 10:49:26 +0200 Subject: [PATCH 9/9] PR review --- cmd/deploy.go | 6 +----- cmd/teardown.go | 6 +----- internal/deployer/deployer.go | 8 ++------ 3 files changed, 4 insertions(+), 16 deletions(-) diff --git a/cmd/deploy.go b/cmd/deploy.go index 6da13b9c..f3ea8695 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -124,11 +124,7 @@ func runDeploy(cmd *cobra.Command, args []string) error { if err != nil { return fmt.Errorf("failed to create deployer: %w", err) } - defer func() { - if err := d.Cleanup(); err != nil { - log.Warningf("Failed to invoke deployer cleanup: %v", err) - } - }() + defer d.Cleanup() if overrideFile != "" { var err error diff --git a/cmd/teardown.go b/cmd/teardown.go index c96b3ea8..2070ab5e 100644 --- a/cmd/teardown.go +++ b/cmd/teardown.go @@ -44,11 +44,7 @@ func runTeardown(cmd *cobra.Command, args []string) error { if err != nil { return fmt.Errorf("failed to create deployer: %w", err) } - defer func() { - if err := d.Cleanup(); err != nil { - log.Warningf("Failed to invoke deployer cleanup: %v", err) - } - }() + defer d.Cleanup() d.SetSingleNamespace(singleNamespace) diff --git a/internal/deployer/deployer.go b/internal/deployer/deployer.go index 007b3e2a..414a54e8 100644 --- a/internal/deployer/deployer.go +++ b/internal/deployer/deployer.go @@ -590,21 +590,17 @@ func (d *Deployer) getClusterResourceKinds() (map[string]struct{}, error) { } // Cleanup cleans up any temporary resources created by the deployer, such as temporary files. -func (d *Deployer) Cleanup() error { +func (d *Deployer) Cleanup() { if d.tempDir != "" && d.envrcFile == "" { // In the case of envrc file usage, we need to keep temporary files around after deployment. // (It contains CA certificates, for example.) if err := os.RemoveAll(d.tempDir); err != nil { - return fmt.Errorf("failed to clean up temporary directory: %w", err) + d.logger.Warningf("Deployer Cleanup failed to remove %q: %v", d.tempDir, err) } } - return nil } // Deploy deploys the specified components to the cluster. -// -// It might create temporary files in the deployer's tempDir, but it is the caller's responsibility to clean them up -// when not used anymore. func (d *Deployer) Deploy(ctx context.Context, components component.Component, resources, exposure string) error { adjustedResources, adjustedExposure, adjustedPortForward := d.clusterDefaults.ApplyConvenienceDefaults( d.kubeContext,