-
Notifications
You must be signed in to change notification settings - Fork 12
Alternative 3 for using WIF to access customer resources for MVP #75
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request adds a new subsection to the WIF spike documentation introducing an MVP-focused approach for Workload Identity Federation. The addition outlines "Alternative 3," a simplified solution where customers grant all identities in a workload identity pool access to resources on their GCP project and Hyperfleet project. The change includes specifications for principalSet permissions, example identifiers, and concrete commands demonstrating authorization across clusters and namespaces using kubectl and gcloud workflows. Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @hyperfleet/docs/wif-spike.md:
- Around line 174-197: The Alternative 3 section (simplest WIF solution) omits a
clear security warning, the concrete gcloud IAM command to grant the
principalSet, and guidance on when this broad-scope approach is appropriate;
update the "Alternative 3" text to (1) insert a prominent security note after
the paragraph describing
principalSet://iam.googleapis.com/projects/275239757837/locations/global/workloadIdentityPools/hcm-hyperfleet.svc.id.goog/*
that states this grants access to all workloads across all clusters/namespaces
in the hcm-hyperfleet project and is high blast-radius (suitable for MVP only),
(2) add the exact gcloud projects add-iam-policy-binding command that binds
roles/pubsub.viewer to the principalSet identifier shown, and (3) add one
sentence clarifying when to use this approach (e.g., MVP/PoC or when additional
network isolation exists) so readers know it is not recommended for production.
- Around line 178-184: The fenced code block showing workload identity
principals is missing a language specifier; update the triple-backtick fence
around the principalSet lines in the wif-spike.md content to include a language
(e.g., use ```text) so the block renders correctly and tooling recognizes it.
- Around line 185-186: Combine the fragmented lines into a single clear sentence
that closes the link and provides context; for example, rewrite the two
fragments so they read like "For example, for a customer project named
`simulated-customer-project-1` (see the console), the project contains a topic
named `sample-topic`," ensuring the parenthesis around the console link are
closed and the sentence is grammatically complete.
- Around line 191-196: Add a language specifier and either specify the namespace
flag or clarify default namespace: update the code block for the kubectl create
job example to start with a bash language tag (```bash) and include the
--namespace=<any-namespace> argument on the kubectl command (referencing the
list-pubsub-topics job example) so the example matches the earlier claim on line
189 that it should succeed in any namespace; alternatively, if default namespace
is intended, add a short note on line 189 clarifying that the command runs in
the default namespace.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
hyperfleet/docs/wif-spike.md
🧰 Additional context used
🪛 LanguageTool
hyperfleet/docs/wif-spike.md
[uncategorized] ~176-~176: Did you mean “Google Docs”?
Context: ...entities in a workload identity pool" ([google docs](https://docs.cloud.google.com/iam/docs...
(GOOGLE_PRODUCTS)
[style] ~177-~177: Consider a more concise word here.
Context: ...fleet` GCP project will be authorized. In order to do this, customer project must allow pe...
(IN_ORDER_TO_PREMIUM)
[style] ~178-~178: Consider a more expressive alternative.
Context: ...roject will be authorized. In order to do this, customer project must allow permi...
(DO_ACHIEVE)
🪛 markdownlint-cli2 (0.18.1)
hyperfleet/docs/wif-spike.md
179-179: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
191-191: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
| ## Alternative 3: Simplest WIF solution for MVP, customer allows all workloads on Hyperfleet GCP project | ||
|
|
||
| For our MVP phase, the simplest solution that works is for the customer to allow "All identities in a workload identity pool" ([google docs](https://docs.cloud.google.com/iam/docs/principal-identifiers#allow)). This means, all the adapter tasks that run in any cluster in the `hcm-hyperfleet` GCP project will be authorized. | ||
|
|
||
| In order to do this, customer project must allow permissions to: | ||
| ``` | ||
| principalSet://iam.googleapis.com/projects/PROJECT_NUMBER/locations/global/workloadIdentityPools/POOL_ID/* | ||
| #for hcm-hyperfleet | ||
| principalSet://iam.googleapis.com/projects/275239757837/locations/global/workloadIdentityPools/hcm-hyperfleet.svc.id.goog/* | ||
| ``` | ||
| As an example, for a customer project named `simulated-customer-project-1` ([link to console](https://console.cloud.google.com/iam-admin/iam?cloudshell=true&project=simulated-customer-project-1) | ||
| It contains a topic named `sample-topic` | ||
|
|
||
| Assign "pubsub viewer permissions" to the principalSet | ||
| The following command will run a k8s job that list the topics in the project. It should succeed in every cluster and any namespace in the `hcm-hyperfleet` project | ||
|
|
||
| ``` | ||
| kubectl create job list-pubsub-topics \ | ||
| --image=google/cloud-sdk:latest \ | ||
| -- \ | ||
| gcloud pubsub topics list --project simulated-customer-project-1 | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Document security implications and provide concrete permission-granting commands.
Alternative 3 grants significantly broader access than the approaches described earlier in the document. Line 64 explicitly states that namespace+ksa scoping is preferred to limit blast radius if tokens leak. This section should:
- Add an explicit security warning noting that all workloads across all clusters and namespaces in the Hyperfleet project gain access to customer resources.
- Provide the actual gcloud command for customers to grant principalSet permissions (similar to lines 38-40 for the granular approach). Currently, the section shows the principalSet identifier but not how to use it.
- Clarify when this approach is appropriate (e.g., "suitable for MVP/proof-of-concept only" or "acceptable when additional network isolation exists").
📝 Suggested additions
After line 177, add a security note:
**Security Note:** This approach grants access to **all workloads** running in any namespace on any cluster within the `hcm-hyperfleet` GCP project. Unlike the namespace+service-account-scoped approach described earlier, a compromised workload in any part of the Hyperfleet infrastructure could access customer resources. This is suitable for MVP but should be refined before production use.After line 184, add the permission-granting command:
# Grant pubsub.viewer role to all identities in the Hyperfleet workload identity pool
gcloud projects add-iam-policy-binding projects/simulated-customer-project-1 \
--role="roles/pubsub.viewer" \
--member="principalSet://iam.googleapis.com/projects/275239757837/locations/global/workloadIdentityPools/hcm-hyperfleet.svc.id.goog/*" \
--condition=None🧰 Tools
🪛 LanguageTool
[uncategorized] ~176-~176: Did you mean “Google Docs”?
Context: ...entities in a workload identity pool" ([google docs](https://docs.cloud.google.com/iam/docs...
(GOOGLE_PRODUCTS)
[style] ~177-~177: Consider a more concise word here.
Context: ...fleet` GCP project will be authorized. In order to do this, customer project must allow pe...
(IN_ORDER_TO_PREMIUM)
[style] ~178-~178: Consider a more expressive alternative.
Context: ...roject will be authorized. In order to do this, customer project must allow permi...
(DO_ACHIEVE)
🪛 markdownlint-cli2 (0.18.1)
179-179: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
191-191: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In @hyperfleet/docs/wif-spike.md around lines 174 - 197, The Alternative 3
section (simplest WIF solution) omits a clear security warning, the concrete
gcloud IAM command to grant the principalSet, and guidance on when this
broad-scope approach is appropriate; update the "Alternative 3" text to (1)
insert a prominent security note after the paragraph describing
principalSet://iam.googleapis.com/projects/275239757837/locations/global/workloadIdentityPools/hcm-hyperfleet.svc.id.goog/*
that states this grants access to all workloads across all clusters/namespaces
in the hcm-hyperfleet project and is high blast-radius (suitable for MVP only),
(2) add the exact gcloud projects add-iam-policy-binding command that binds
roles/pubsub.viewer to the principalSet identifier shown, and (3) add one
sentence clarifying when to use this approach (e.g., MVP/PoC or when additional
network isolation exists) so readers know it is not recommended for production.
| In order to do this, customer project must allow permissions to: | ||
| ``` | ||
| principalSet://iam.googleapis.com/projects/PROJECT_NUMBER/locations/global/workloadIdentityPools/POOL_ID/* | ||
| #for hcm-hyperfleet | ||
| principalSet://iam.googleapis.com/projects/275239757837/locations/global/workloadIdentityPools/hcm-hyperfleet.svc.id.goog/* | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add language specifier to code block.
The fenced code block is missing a language specifier, which affects rendering and documentation tooling.
📝 Suggested fix
-```
+```text
principalSet://iam.googleapis.com/projects/PROJECT_NUMBER/locations/global/workloadIdentityPools/POOL_ID/*
#for hcm-hyperfleet
principalSet://iam.googleapis.com/projects/275239757837/locations/global/workloadIdentityPools/hcm-hyperfleet.svc.id.goog/*</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
In order to do this, customer project must allow permissions to:
🧰 Tools
🪛 LanguageTool
[style] ~178-~178: Consider a more expressive alternative.
Context: ...roject will be authorized. In order to do this, customer project must allow permi...
(DO_ACHIEVE)
🪛 markdownlint-cli2 (0.18.1)
179-179: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In @hyperfleet/docs/wif-spike.md around lines 178 - 184, The fenced code block
showing workload identity principals is missing a language specifier; update the
triple-backtick fence around the principalSet lines in the wif-spike.md content
to include a language (e.g., use ```text) so the block renders correctly and
tooling recognizes it.
| As an example, for a customer project named `simulated-customer-project-1` ([link to console](https://console.cloud.google.com/iam-admin/iam?cloudshell=true&project=simulated-customer-project-1) | ||
| It contains a topic named `sample-topic` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improve sentence structure.
The sentence is fragmented across these lines. Consider restructuring for clarity.
📝 Suggested fix
-As an example, for a customer project named `simulated-customer-project-1` ([link to console](https://console.cloud.google.com/iam-admin/iam?cloudshell=true&project=simulated-customer-project-1)
-It contains a topic named `sample-topic`
+For example, consider a customer project named `simulated-customer-project-1` ([link to console](https://console.cloud.google.com/iam-admin/iam?cloudshell=true&project=simulated-customer-project-1)) that contains a topic named `sample-topic`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| As an example, for a customer project named `simulated-customer-project-1` ([link to console](https://console.cloud.google.com/iam-admin/iam?cloudshell=true&project=simulated-customer-project-1) | |
| It contains a topic named `sample-topic` | |
| For example, consider a customer project named `simulated-customer-project-1` ([link to console](https://console.cloud.google.com/iam-admin/iam?cloudshell=true&project=simulated-customer-project-1)) that contains a topic named `sample-topic`. |
🤖 Prompt for AI Agents
In @hyperfleet/docs/wif-spike.md around lines 185 - 186, Combine the fragmented
lines into a single clear sentence that closes the link and provides context;
for example, rewrite the two fragments so they read like "For example, for a
customer project named `simulated-customer-project-1` (see the console), the
project contains a topic named `sample-topic`," ensuring the parenthesis around
the console link are closed and the sentence is grammatically complete.
| ``` | ||
| kubectl create job list-pubsub-topics \ | ||
| --image=google/cloud-sdk:latest \ | ||
| -- \ | ||
| gcloud pubsub topics list --project simulated-customer-project-1 | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add language specifier and clarify namespace usage.
The code block has two issues:
- Missing language specifier (affects documentation rendering)
- The command doesn't specify a namespace, but line 189 claims it "should succeed in every cluster and any namespace"
📝 Suggested fix
-```
+```bash
kubectl create job list-pubsub-topics \
+ --namespace=<any-namespace> \
--image=google/cloud-sdk:latest \
-- \
gcloud pubsub topics list --project simulated-customer-project-1
Alternatively, if using the default namespace is intentional, clarify on line 189:
```diff
-The following command will run a k8s job that list the topics in the project. It should succeed in every cluster and any namespace in the `hcm-hyperfleet` project
+The following command will run a k8s job that lists the topics in the project. It should succeed in every cluster and any namespace in the `hcm-hyperfleet` project (replace `--namespace=default` with any namespace to test):
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ``` | |
| kubectl create job list-pubsub-topics \ | |
| --image=google/cloud-sdk:latest \ | |
| -- \ | |
| gcloud pubsub topics list --project simulated-customer-project-1 | |
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
191-191: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In @hyperfleet/docs/wif-spike.md around lines 191 - 196, Add a language
specifier and either specify the namespace flag or clarify default namespace:
update the code block for the kubectl create job example to start with a bash
language tag (```bash) and include the --namespace=<any-namespace> argument on
the kubectl command (referencing the list-pubsub-topics job example) so the
example matches the earlier claim on line 189 that it should succeed in any
namespace; alternatively, if default namespace is intended, add a short note on
line 189 clarifying that the command runs in the default namespace.
Added info from this thread
https://redhat-internal.slack.com/archives/C0916E39DQV/p1767783594738609?thread_ts=1767633114.191379&cid=C0916E39DQV
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.