Allow to process GCS File in Profile Section #9864

ashokhein · 2025-09-05T10:35:02Z

Description

We are now processing GCS Object/Value File references during the deploy step. To support this, the envTemplate logic has been rearranged so that environment variables are resolved before extracting GCS files.

However, the previous approach did not work for --values= references defined in the Profile section. To fix this, new code has been added to handle GCS references there as well.

profiles:
  - name: k8s
    activation:
      - env: ENVIRONMENT_TYPE=k8s
    patches:    
      - op: "add"
        path: "/deploy/helm/flags/install/-" # The "-" means to add at the end of the list
        value: "--values=gs://xxxk8s/infra.yaml"
      - op: "add"
        path: "/deploy/helm/flags/upgrade/-" # The "-" means to add at the end of the list
        value: "--values=gs://xxxk8s/infra.yaml"

This will convert to the following Helm Args
helm --kube-context gke_xx_b3 install release-xxx-backend --values=/var/folders/zd/js5hcjy968b_j_3zn9bt3r9m0000gp/T/helm_values_from_gcs1737245839/infra.yaml

gemini-code-assist

Summary of Changes

Hello @ashokhein, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances Skaffold's Helm deployer to support referencing Helm values files stored in Google Cloud Storage (GCS) buckets, specifically when defined within Skaffold's profile sections. This allows for centralized management of environment-specific configurations by automatically downloading GCS-hosted value files before Helm commands are executed.

Highlights

GCS Support in Helm Profiles: Skaffold now processes GCS object references (e.g., gs://bucket/file.yaml) within Helm --values flags and valuesFiles defined in profiles sections of skaffold.yaml.
Automated GCS File Resolution: The system automatically detects GCS URLs, downloads the files to temporary local paths, and passes these local paths to Helm commands, cleaning up afterwards.
Documentation Updates: Comprehensive documentation has been added to the Helm deployer and profiles sections, providing clear examples for using GCS references.
Code Refactoring for Reusability: The internal extractValueFileFromGCS function has been made public (ExtractValueFileFromGCS) to facilitate its reuse across different parts of the codebase.
New GCS Flag Processing Logic: A new function processGCSFlags was introduced to specifically handle GCS URLs embedded within Helm --values or -f flags.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces a valuable feature by allowing GCS file references in Helm profile patches, and the core logic and documentation updates look good. However, my main concern is the introduction of resource leaks. Both the new processGCSFlags function and the modified ConstructOverrideArgs create temporary directories that are never cleaned up, which will lead to an accumulation of files on the user's machine. I've flagged these as high-severity issues. Additionally, I've noted some minor issues in the documentation and tests that should be addressed for better code quality. Once the resource leaks are fixed, this will be a solid contribution.

gemini-code-assist · 2025-09-05T10:37:50Z

pkg/skaffold/deploy/helm/args.go

+func processGCSFlags(flags []string) ([]string, error) {
+	if len(flags) == 0 {
+		return flags, nil
+	}
+
+	var processedFlags []string
+	gcs := gcs.NewGsutil()
+
+	for i := 0; i < len(flags); i++ {
+		flag := flags[i]
+
+		// Check for --values flag with equals sign (--values=gs://...)
+		if strings.HasPrefix(flag, "--values=") {
+			value := strings.TrimPrefix(flag, "--values=")
+			if strings.HasPrefix(value, "gs://") {
+				tempDir, err := os.MkdirTemp("", "helm_values_from_gcs")
+				if err != nil {
+					return nil, fmt.Errorf("failed to create temp directory: %w", err)
+				}
+				processedValue, err := extractValueFileFromGCSFunc(value, tempDir, gcs)
+				if err != nil {
+					return nil, err
+				}
+				processedFlags = append(processedFlags, "--values="+processedValue)
+			} else {
+				processedFlags = append(processedFlags, flag)
+			}
+		} else if flag == "--values" || flag == "-f" {
+			// Check for --values flag with separate argument (--values gs://... or -f gs://...)
+			if i+1 < len(flags) {
+				nextFlag := flags[i+1]
+				if strings.HasPrefix(nextFlag, "gs://") {
+					tempDir, err := os.MkdirTemp("", "helm_values_from_gcs")
+					if err != nil {
+						return nil, fmt.Errorf("failed to create temp directory: %w", err)
+					}
+					processedValue, err := extractValueFileFromGCSFunc(nextFlag, tempDir, gcs)
+					if err != nil {
+						return nil, err
+					}
+					processedFlags = append(processedFlags, flag, processedValue)
+					i++ // Skip the next flag since we processed it
+				} else {
+					processedFlags = append(processedFlags, flag, nextFlag)
+					i++
+				}
+			} else {
+				processedFlags = append(processedFlags, flag)
+			}
+		} else {
+			processedFlags = append(processedFlags, flag)
+		}
+	}
+
+	return processedFlags, nil
+}


The function processGCSFlags creates temporary directories using os.MkdirTemp for each GCS values file, but it doesn't arrange for them to be cleaned up. This will result in a resource leak, as temporary directories will accumulate on the user's machine. These directories should be cleaned up after the helm command that uses them has finished.

A common pattern in Skaffold is to have the Deployer manage the cleanup of temporary resources. I recommend modifying processGCSFlags to return the paths of all temporary directories it creates. The caller (installArgs) can then add these to a list on the Deployer, and the Deployer.Cleanup method can be updated to remove them.

gemini-code-assist · 2025-09-05T10:37:50Z

pkg/skaffold/helm/args.go

 			tempDir, err := os.MkdirTemp("", valueFileFromGCS)
 			if err != nil {
 				return nil, fmt.Errorf("failed to create the tmp directory: %w", err)
 			}


A temporary directory is created here with os.MkdirTemp but is never cleaned up. This will lead to a resource leak on the user's machine. This appears to be a pre-existing issue, but since this code block is modified, it's a good opportunity to fix it. The caller of ConstructOverrideArgs should be made responsible for cleaning up these directories after they are no longer needed. This might require ConstructOverrideArgs to return the list of temporary directories it creates, similar to the suggestion for processGCSFlags.

gemini-code-assist · 2025-09-05T10:37:50Z

docs-v2/content/en/docs/environment/profiles.md

+
+      # Or add GCS reference as --values flag
+      - op: add
+        path: /deploy/helm/flags/install/-
+        value: "--values=gs://my-config-bucket/prod-values.yaml"
+      - op: add  
+        path: /deploy/helm/flags/upgrade/-
+        value: "--values=gs://my-config-bucket/prod-values.yaml"
+


There are some minor formatting issues in this YAML example, such as trailing whitespace on empty lines and extra spaces. Cleaning this up will improve the readability and consistency of the documentation.

Suggested change

# Or add GCS reference as --values flag

- op: add

path: /deploy/helm/flags/install/-

value: "--values=gs://my-config-bucket/prod-values.yaml"

- op: add

path: /deploy/helm/flags/upgrade/-

value: "--values=gs://my-config-bucket/prod-values.yaml"

# Or add GCS reference as --values flag

- op: add

path: /deploy/helm/flags/install/-

value: "--values=gs://my-config-bucket/prod-values.yaml"

- op: add

path: /deploy/helm/flags/upgrade/-

value: "--values=gs://my-config-bucket/prod-values.yaml"

gemini-code-assist · 2025-09-05T10:37:50Z

pkg/skaffold/deploy/helm/args_test.go

+						if !strings.HasPrefix(value, "/") || !strings.HasSuffix(value, ".yaml") {
+							t.Errorf("expected temp file path, got: %s", value)
+						}
+					} else if expectedFlag == "/tmp/test-file.yaml" {
+						// For separate flags, check that it's a temp file path
+						if !strings.HasPrefix(flag, "/") || !strings.HasSuffix(flag, ".yaml") {
+							t.Errorf("expected temp file path, got: %s", flag)
+						}


The path checks using strings.HasPrefix(..., "/") are not platform-agnostic and will fail on Windows, where absolute paths do not start with /. Please use filepath.IsAbs() for a portable solution to check for absolute paths.

ashokhein added 11 commits November 19, 2023 00:39

feat: allow ValuesFile from GCS

da9aa04

Merge branch 'GoogleContainerTools:main' into main

a9af349

Merge branch 'GoogleContainerTools:main' into main

1bab52c

added extracted gcs function

ce8b80c

Merge branch 'GoogleContainerTools:main' into main

aa78d3b

added test cases for copy gcs func

0d1f8bc

only create an tmp directory for gcs file

1608c21

fixed formatting

9180d22

Merge branch 'GoogleContainerTools:main' into main

ea6dcdf

Processing Flags to Check GCS References

fd1b81c

updating documentation

41bfe02

ashokhein requested a review from a team as a code owner September 5, 2025 10:35

pull-request-size bot added the size/XL label Sep 5, 2025

gemini-code-assist bot reviewed Sep 5, 2025

View reviewed changes

updated filepath to use abs

94f3280

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Allow to process GCS File in Profile Section #9864

Allow to process GCS File in Profile Section #9864

Uh oh!

ashokhein commented Sep 5, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Sep 5, 2025

Uh oh!

gemini-code-assist bot Sep 5, 2025

Uh oh!

gemini-code-assist bot Sep 5, 2025

Uh oh!

gemini-code-assist bot Sep 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Allow to process GCS File in Profile Section #9864

Are you sure you want to change the base?

Allow to process GCS File in Profile Section #9864

Uh oh!

Conversation

ashokhein commented Sep 5, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant