Skip to content

fix(blueprints): apply Workbench notebook owners - #338

Merged
aghassemlouei merged 1 commit into
google:mainfrom
huong8373tt-beep:fix/317-workbench-instance-owner
Oct 22, 2026
Merged

aghassemlouei merged 1 commit into
google:mainfrom
huong8373tt-beep:fix/317-workbench-instance-owner

Conversation

@huong8373tt-beep

@huong8373tt-beep huong8373tt-beep commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Description

Fixes #317.

var.notebooks[each.key].owner is a string, whereas google_workbench_instance.playground expects instance_owners to be a list(string). Calling try(tolist(var.notebooks[each.key].owner), null) in blueprints/fedramp-high/vertex-mlops/vertex.tf failed type conversion on strings and always evaluated to null. This change passes null when owner is unset and [var.notebooks[each.key].owner] when configured, and documents the optional owner setting for USER_MANAGED notebooks in README.md and terraform.tfvars.sample.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

Deployment & Compliance Impact

  • Applicable Regimes:
    • US Region Restricted (e.g., Access Policy constraint)
    • FedRAMP Moderate
    • FedRAMP High
    • DoD IL4
    • DoD IL5
    • General / All
  • NIST 800-53r5 Controls: AC-2, AC-3

Checklist

Code Quality & Reusability

  • My code adheres to the Maximize Reusability principle. I have not redefined common elements and have reused existing base configurations and modules where possible.
  • I have checked that no existing module or configuration in modules/ or fast/stages-aw/ can be leveraged for this change.
  • My code follows the established naming conventions outlined in docs/naming-convention.md.

Documentation

  • I have updated the README.md of the modified module or blueprint.
  • I have added/updated documentation for inputs (variables) and outputs.

Security

  • My change adheres to GCP security best practices and the principle of least privilege.
  • I have ensured compliance with the targeted regime (FedRAMP Moderate, FedRAMP High, IL4, IL5, etc.).

Testing

  • I have tested my changes locally.
  • I have included details of my testing in this PR.

Testing Performed

  • Executed bash tools/run_tests.sh and bash tools/lint.sh.
  • Verified with terraform init and terraform plan in blueprints/fedramp-high/vertex-mlops that google_workbench_instance.playground resolves instance_owners to [<owner>] when owner is set and null when omitted.

@github-actions github-actions Bot added Framework - FedRAMP High FedRAMP High compliance regime and controls Scope - Blueprint Related to solution blueprints in the blueprints directory labels Oct 22, 2026
@google-cla

google-cla Bot commented Oct 22, 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.

@aghassemlouei
aghassemlouei force-pushed the fix/317-workbench-instance-owner branch from 5937f39 to c0ffa77 Compare October 22, 2026 14:54
@aghassemlouei aghassemlouei added Bug Something isn't working Security Something is insecure or can be secured Priority - Medium Standard features and non-blocking bugs; important for the current milestone but not urgent Level of Effort - Low Quick, well-defined tasks with no unknowns; takes a few hours up to one day to complete labels Oct 22, 2026
@aghassemlouei
aghassemlouei force-pushed the fix/317-workbench-instance-owner branch from c0ffa77 to 5937f39 Compare October 22, 2026 15:09
@aghassemlouei aghassemlouei added the Documentation Improvements or additions to documentation label Oct 22, 2026

@aghassemlouei aghassemlouei left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for catching and fixing the Workbench owner list conversion in vertex-mlops, @huong8373tt-beep! Validated the plan output and this looks good to merge.

@aghassemlouei
aghassemlouei merged commit fc19ec4 into google:main Oct 22, 2026
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something isn't working Documentation Improvements or additions to documentation Framework - FedRAMP High FedRAMP High compliance regime and controls Level of Effort - Low Quick, well-defined tasks with no unknowns; takes a few hours up to one day to complete Priority - Medium Standard features and non-blocking bugs; important for the current milestone but not urgent Scope - Blueprint Related to solution blueprints in the blueprints directory Security Something is insecure or can be secured

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] fedramp-high/vertex-mlops: the notebook owner is never applied to Workbench instances

2 participants