Skip to content

fix: return cloudrun-sandbox defaults on single-node tier - #1481

Merged
ptone merged 2 commits into
GoogleCloudPlatform:mainfrom
ptone:scion/sn-profile-defaults-dev
Sep 7, 2026
Merged

fix: return cloudrun-sandbox defaults on single-node tier#1481
ptone merged 2 commits into
GoogleCloudPlatform:mainfrom
ptone:scion/sn-profile-defaults-dev

Conversation

@ptone

@ptone ptone commented Sep 7, 2026

Copy link
Copy Markdown
Member

Summary

  • GetDefaultSettingsDataYAML() always returned the workstation template (default_settings.yaml) which defines local (docker) and remote (kubernetes) profiles
  • On the Cloud Run single-node tier, this caused "remote (kubernetes)" to appear as the only runtime profile since the local profile gets filtered out
  • Added isCloudRunSandboxEnvironment() check to return the cloudrun-sandbox template instead

Changes

  • pkg/config/koanf.go: 7-line addition at top of GetDefaultSettingsDataYAML()
  • pkg/config/koanf_test.go: 3 new test functions (131 lines) covering positive, negative, and integration scenarios

Test plan

  • All pkg/config tests pass including existing TestInitMachine_CloudRunSandbox_* tests
  • New tests verify cloudrun-sandbox defaults returned when env conditions met
  • New tests verify workstation defaults still returned when conditions NOT met
  • Integration test verifies LoadVersionedSettings produces correct profiles on cloudrun-sandbox tier

Tracking: ptone#1448

GetDefaultSettingsDataYAML() always returned the workstation template
(local/docker + remote/kubernetes profiles), even on Cloud Run Instance
with sandbox launcher. This caused the UI to show "remote (kubernetes)"
as the only available profile on the single-node tier.

Add isCloudRunSandboxEnvironment() check at the top of the function so
it returns default_settings_cloudrun_sandbox.yaml when running on that
tier. The function already existed in the same package (init.go:685).

Fixes #1448
@google-cla

google-cla Bot commented Sep 7, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates GetDefaultSettingsDataYAML in pkg/config/koanf.go to return the tier-specific Cloud Run Sandbox settings template when running in that environment, and adds corresponding unit tests. However, the reviewer points out that this fix is incomplete because LoadSettingsKoanf relies on GetDefaultSettingsData in pkg/config/init.go, which does not check for the Cloud Run Sandbox environment and will still merge workstation profiles in production.

Comment thread pkg/config/koanf.go
Comment on lines +278 to +280
if isCloudRunSandboxEnvironment() {
return EmbedsFS.ReadFile("embeds/default_settings_cloudrun_sandbox.yaml")
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

While fixing GetDefaultSettingsDataYAML() correctly addresses the defaults for LoadVersionedSettings(), the main settings loader LoadSettingsKoanf() (defined in koanf.go line 53) calls GetDefaultSettingsData() (defined in init.go line 84) to load the initial embedded defaults.

Currently, GetDefaultSettingsData() does not check isCloudRunSandboxEnvironment() and always returns the workstation defaults (with docker or container runtime) converted to JSON. Because Koanf merges maps, calling LoadSettingsKoanf() in a Cloud Run Sandbox environment will still result in the workstation profiles (local, remote) and runtimes (docker, kubernetes, etc.) being merged into the final Settings struct, even if the global settings file only defines the default profile.

This is also why the new integration test TestGetDefaultSettingsDataYAML_CloudRunSandbox_ProfilesCorrect passes (since it uses LoadVersionedSettings), but the actual runtime configuration loaded via LoadSettingsKoanf in production remains polluted with workstation profiles.

To fully resolve this, GetDefaultSettingsData() in pkg/config/init.go should also be updated to handle the Cloud Run Sandbox environment, converting the sandbox YAML template to JSON.

…y path)

GetDefaultSettingsDataYAML (used by LoadVersionedSettings) already checks
isCloudRunSandboxEnvironment() and returns the cloudrun-sandbox template.
However, GetDefaultSettingsData (used by LoadSettingsKoanf) did not — the
legacy settings loading path still returned workstation defaults on the
cloudrun-sandbox tier. This addresses the Gemini HIGH review finding on
upstream PR GoogleCloudPlatform#1481.

The fix adds the same isCloudRunSandboxEnvironment() guard to
GetDefaultSettingsData(), routing to the cloudrun-sandbox template and
converting it to the legacy JSON format via a new defaultSettingsYAMLToJSON
helper. Also corrects a stale comment in koanf.go that said
GetDefaultSettingsDataYAML is called by LoadSettingsKoanf (it is called by
LoadVersionedSettings).
@ptone
ptone merged commit af60f21 into GoogleCloudPlatform:main Sep 7, 2026
10 of 12 checks passed
@ptone
ptone deleted the scion/sn-profile-defaults-dev branch September 7, 2026 05:09
ptone added a commit that referenced this pull request Sep 7, 2026
* docs: add npm registry/proxy section to custom-images guide

Document the NPM_REGISTRY build arg and NPM_CONFIG_FILE BuildKit secret
introduced in #1476, which enable image builds behind corporate proxies
where registry.npmjs.org is blocked.

Changelog: 2026-09-06
Other changelog items reviewed — no further docs impact:
- #1478, #1480, #1483, #1481, #1479: internal fixes
- #1482: host SA auto-detection (docs had no manual step to remove)
- #1475: Azure DevOps URL parsing fix (no interface change)
- #1474: UI fix; #1473: demo script fix

* docs: add weekly release notes for Aug 31 - Sep 6, 2026

---------

Co-authored-by: Scion Agent (du-0906) <agent@scion.dev>
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.

1 participant