LOG-7348: Add WIF support to CLO for googleCloud output by cahartma · Pull Request #3243 · openshift/cluster-logging-operator

cahartma · 2026-04-02T20:15:03Z

Description

Simple CLO changes to allow googeCloud WIF authentication. WIF functionality will also require an update to the vector client. The CLO changes are backwards compatible with existing vector auth functionality.

I've included a design document to highlight the API decision. There are no changes to the googleCloud authentication API. Service account tokens are always projected for GCP outputs to support WIF. This does not present any performance or security issues and greatly simplifies the implementation.

Users provide an authentication secret reference pointing to a credentials file:

type: "service_account" (current static authentication)
OR
type: "external_account" (Workload Identity Federation)

Vector detects the type of file and handles the appropriate authentication flow.

/cc @Clee2691 @vparfonov
/assign @jcantrill

Links

CLO (this PR): https://redhat.atlassian.net/browse/LOG-7348
Vector update: https://redhat.atlassian.net/browse/LOG-9171
Epic: https://redhat.atlassian.net/browse/LOG-3577

openshift-ci-robot · 2026-04-02T20:15:09Z

@cahartma: This pull request references LOG-7348 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.8.0" version, but no target version was set.

Details

In response to this:

Description

Simple CLO changes to allow googeCloud WIF authentication. WIF functionality will also require an update to the vector client. The CLO changes are backwards compatible with existing vector auth functionality.

I've included a design document to highlight the API decision. There are no changes to the googleCloud authentication API. Service account tokens are always projected for GCP outputs to support WIF. This does not present any performance or security issues and greatly simplifies the implementation.

Users provide an authentication secret reference pointing to a credentials file:

type: "service_account" (current static authentication)
OR

type: "external_account" (Workload Identity Federation)

Vector detects the type of file and handles the appropriate authentication flow.

/cc @Clee2691 @vparfonov
/assign @jcantrill

Links

CLO (this PR): https://redhat.atlassian.net/browse/LOG-7348

Vector update: https://redhat.atlassian.net/browse/LOG-9171

Epic: https://redhat.atlassian.net/browse/LOG-3577

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

qodo-code-review · 2026-04-02T20:15:20Z

Review Summary by Qodo

Add Workload Identity Federation support for Google Cloud Logging output

✨ Enhancement

Walkthroughs

Description

• Add Workload Identity Federation (WIF) support for GCP authentication
• Simplify API by auto-detecting credentials file type at runtime
• Always project service account token for GCP outputs
• Refactor bearer token extraction logic for maintainability
• Add comprehensive WIF configuration and troubleshooting documentation

Diagram

flowchart LR
  A["GoogleCloudLogging<br/>Authentication"] -->|Credentials File| B["Runtime Detection"]
  B -->|service_account type| C["Static Key Auth"]
  B -->|external_account type| D["WIF Auth"]
  D -->|Uses Projected Token| E["STS Token Exchange"]
  E -->|Impersonates| F["GCP Service Account"]
  C -->|Direct Access| F
  F -->|Logs| G["Cloud Logging API"]

File Changes

1. api/observability/v1/output_types.go 📝 Documentation +4/-1

Update GoogleCloudLogging credentials documentation

api/observability/v1/output_types.go

2. internal/api/observability/configmap_test.go Formatting +32/-32

Fix test indentation formatting

internal/api/observability/configmap_test.go

3. internal/api/observability/output_types.go ✨ Enhancement +49/-22

Refactor service account token detection logic

internal/api/observability/output_types.go

View more (11)

4. internal/api/observability/output_types_test.go 🧪 Tests +99/-0

Add comprehensive GCP and bearer token tests

internal/api/observability/output_types_test.go

5. bundle/manifests/cluster-logging.clusterserviceversion.yaml 📝 Documentation +6/-2

Update CSV credentials documentation and timestamp

bundle/manifests/cluster-logging.clusterserviceversion.yaml

6. bundle/manifests/observability.openshift.io_clusterlogforwarders.yaml 📝 Documentation +5/-2

Update CRD credentials field documentation

bundle/manifests/observability.openshift.io_clusterlogforwarders.yaml

7. config/crd/bases/observability.openshift.io_clusterlogforwarders.yaml 📝 Documentation +5/-2

Update CRD credentials field documentation

config/crd/bases/observability.openshift.io_clusterlogforwarders.yaml

8. config/manifests/bases/cluster-logging.clusterserviceversion.yaml 📝 Documentation +5/-1

Update CSV credentials documentation

config/manifests/bases/cluster-logging.clusterserviceversion.yaml

9. docs/features/logforwarding/outputs/googlecloud/gcp-authentication-design.adoc 📝 Documentation +155/-0

Add GCP authentication design document

docs/features/logforwarding/outputs/googlecloud/gcp-authentication-design.adoc

10. docs/features/logforwarding/outputs/googlecloud/google-cloud-workload-identity.adoc 📝 Documentation +568/-0

Add comprehensive WIF configuration guide

docs/features/logforwarding/outputs/googlecloud/google-cloud-workload-identity.adoc

11. docs/features/logforwarding/outputs/aws/aws-cross-account-forwarding.adoc Additional files +0/-0

...

docs/features/logforwarding/outputs/aws/aws-cross-account-forwarding.adoc

12. docs/features/logforwarding/outputs/aws/cloudwatch-sts-forwarding.adoc Additional files +0/-0

...

docs/features/logforwarding/outputs/aws/cloudwatch-sts-forwarding.adoc

13. docs/features/logforwarding/outputs/aws/s3-forwarding.adoc Additional files +0/-0

...

docs/features/logforwarding/outputs/aws/s3-forwarding.adoc

14. docs/features/logforwarding/outputs/googlecloud/google-cloud-forwarding.adoc Additional files +0/-0

...

docs/features/logforwarding/outputs/googlecloud/google-cloud-forwarding.adoc

qodo-code-review · 2026-04-02T20:15:21Z

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX Issues (0)

1. Unnecessary SA token Secret 🐞 Bug ⛨ Security

Description

NeedServiceAccountToken() now returns true for GoogleCloudLogging outputs, which causes
ReconcileCollector to create and mount a SecretTypeServiceAccountToken even though the GCP Vector
sink only uses credentials_path. This introduces an extra long-lived service account token Secret
for GCP-only configurations, increasing token exposure without being used for GCP auth.

Code

internal/api/observability/output_types.go[R56-66]

+// needsServiceAccountToken returns true if the output requires service account token projection
+func needsServiceAccountToken(o obsv1.OutputSpec) bool {
+	// For GCP we always project the service account token
+	if o.Type == obsv1.OutputTypeGoogleCloudLogging && o.GoogleCloudLogging != nil && o.GoogleCloudLogging.Authentication != nil {
+		return true
+	}
+
+	// If any output has explicit service account token config
+	token := getOutputBearerToken(o)
+	return token != nil && token.From == obsv1.BearerTokenFromServiceAccount
+}

Evidence
The PR change makes GoogleCloudLogging outputs require NeedServiceAccountToken(), which is used by
the controller to create a legacy ServiceAccount token Secret (non-projected). The GCP sink
generator only configures credentials_path from the credentials Secret, and does not reference the
serviceAccountTokenSecretName option, so that legacy token Secret is unused for GCP.
internal/api/observability/output_types.go[56-66]
internal/controller/observability/collector.go[49-64]
internal/auth/service_account.go[34-59]
internal/generator/vector/output/gcl/gcl.go[35-58]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`Outputs.NeedServiceAccountToken()` is now true for GCP outputs to support WIF token *projection*, but the controller also uses this same flag to create a `SecretTypeServiceAccountToken` (legacy, long-lived) and mount it into collector pods. The GCP sink configuration only needs the *projected* token file (and only uses it when the credentials file is `external_account`), so creating the legacy token Secret for GCP-only setups is unnecessary exposure.

### Issue Context
- Token projection is added at the pod-spec level when `NeedServiceAccountToken()` is true.
- The controller additionally creates a legacy SA token Secret when `NeedServiceAccountToken()` is true.
- Vector GCP sink uses only `credentials_path` (credentials Secret), not the legacy token Secret option.

### Fix Focus Areas
- internal/api/observability/output_types.go[46-97]
- internal/controller/observability/collector.go[49-64]
- internal/auth/service_account.go[34-59]

### Suggested approach
1. Split the concept into two checks:
  - `NeedServiceAccountTokenProjection()` (true for AWS IAMRole and GCP WIF support, etc.)
  - `NeedServiceAccountTokenSecret()` (true only when a generator actually needs `OptionServiceAccountTokenSecretName`, e.g., `BearerTokenFromServiceAccount` HTTP auth cases).
2. Keep using the projection check to add the projected volume.
3. In the controller, create `SecretTypeServiceAccountToken` **only** when `NeedServiceAccountTokenSecret()` is true.
4. Add/adjust unit tests to cover “GCP only => projection true, token Secret false” behavior.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. Misleading “always project” comment 🐞 Bug ⚙ Maintainability

Description

needsServiceAccountToken() claims GCP always projects the service account token, but the
implementation only returns true when googleCloudLogging.authentication is non-nil. This mismatch
makes the intent unclear and increases the risk of future maintenance errors.

Code

internal/api/observability/output_types.go[R56-61]

+// needsServiceAccountToken returns true if the output requires service account token projection
+func needsServiceAccountToken(o obsv1.OutputSpec) bool {
+	// For GCP we always project the service account token
+	if o.Type == obsv1.OutputTypeGoogleCloudLogging && o.GoogleCloudLogging != nil && o.GoogleCloudLogging.Authentication != nil {
+		return true
+	}

Evidence
The comment states unconditional behavior for GCP, but the code requires Authentication to be
non-nil; the API schema also allows GoogleCloudLogging.Authentication to be omitted, making the
comment concretely inaccurate.
internal/api/observability/output_types.go[56-61]
api/observability/v1/output_types.go[595-601]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The comment says GCP outputs “always” project a service account token, but the code only does so when `GoogleCloudLogging.Authentication != nil`.

### Issue Context
This is a readability/intent issue that can cause confusion when changing auth behavior later.

### Fix Focus Areas
- internal/api/observability/output_types.go[56-61]

### Suggested fix
Update the comment to match the condition (e.g., “For GCP outputs with authentication configured…”), or remove the `Authentication != nil` condition if the intent truly is unconditional projection for all GCP outputs.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

ⓘ The new review experience is currently in Beta. Learn more

openshift-ci · 2026-04-02T20:25:49Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cahartma

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [cahartma]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci · 2026-04-02T22:59:01Z

@cahartma: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

jcantrill · 2026-04-06T09:33:53Z

/hold

jcantrill · 2026-04-06T09:40:31Z

+
+The GCP authentication API uses a *simplified, credentials-only approach*. The authentication type (static token vs. Workload Identity Federation) is determined by *inspecting the credentials file content* at runtime, rather than requiring users to specify an explicit type field.
+
+== API Structure


Consider removing this section since it is an implementation detail that has no front facing purpose?

jcantrill · 2026-04-06T09:43:32Z

+
+*Requirements*:
+
+* The external_account.json must reference the token file path: `/var/run/ocp-collector/serviceaccount/token`


Do we validate this during reconciliation? If not, we should. Consider adding a note that we validate and failed validation will keep the collector from deploying

jcantrill · 2026-04-06T09:46:16Z

+. *Secrets may not exist yet* - Users may have created the ClusterLogForwarder before creating secrets
+. *No file inspection in validation* - Validation code should not need to read secret contents
+. *Simplicity* - Eliminates complex file parsing, error handling, and caching logic
+. *No security impact* - Projecting an unused token is harmless; Vector only uses it if needed


s/Vector/the collector

jcantrill · 2026-04-06T09:46:43Z

+*Rationale*:
+
+. *Secrets may not exist yet* - Users may have created the ClusterLogForwarder before creating secrets
+. *No file inspection in validation* - Validation code should not need to read secret contents


we should consider validating the content for the correct path

jcantrill · 2026-04-06T09:47:40Z

+
+This detection happens *at runtime in the Vector collector client*, which  supports both authentication methods through the same `credentials_path` configuration.
+
+=== Vector Configuration


Consider removing this as well since it is an implementation detail

LOG-7348: Add WIF support for googleCloud output

22eed78

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Apr 2, 2026

openshift-ci bot assigned jcantrill Apr 2, 2026

openshift-ci bot requested review from Clee2691 and vparfonov April 2, 2026 20:16

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 2, 2026

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 6, 2026

jcantrill added the release/6.6 label Apr 6, 2026

jcantrill reviewed Apr 6, 2026

View reviewed changes

jcantrill added a commit to jcantrill/cluster-logging-operator that referenced this pull request Apr 16, 2026

fixes openshift#3243. fixes README dead link

dd6cd1e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LOG-7348: Add WIF support to CLO for googleCloud output#3243

LOG-7348: Add WIF support to CLO for googleCloud output#3243
cahartma wants to merge 1 commit intoopenshift:masterfrom
cahartma:LOG-7348-gcp-wif-support

cahartma commented Apr 2, 2026

Uh oh!

openshift-ci-robot commented Apr 2, 2026 •

edited by openshift-ci bot

Loading

Description

Links

Uh oh!

qodo-code-review bot commented Apr 2, 2026

Uh oh!

qodo-code-review bot commented Apr 2, 2026 •

edited

Loading

Uh oh!

openshift-ci bot commented Apr 2, 2026

Uh oh!

openshift-ci bot commented Apr 2, 2026

Uh oh!

jcantrill commented Apr 6, 2026

Uh oh!

jcantrill Apr 6, 2026

Uh oh!

jcantrill Apr 6, 2026

Uh oh!

jcantrill Apr 6, 2026

Uh oh!

jcantrill Apr 6, 2026

Uh oh!

jcantrill Apr 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		The GCP authentication API uses a simplified, credentials-only approach. The authentication type (static token vs. Workload Identity Federation) is determined by inspecting the credentials file content at runtime, rather than requiring users to specify an explicit type field.

		== API Structure


		Requirements:

		* The external_account.json must reference the token file path: `/var/run/ocp-collector/serviceaccount/token`


		This detection happens at runtime in the Vector collector client, which supports both authentication methods through the same `credentials_path` configuration.

		=== Vector Configuration

Conversation

cahartma commented Apr 2, 2026

Description

Links

Uh oh!

openshift-ci-robot commented Apr 2, 2026 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Links

Uh oh!

qodo-code-review bot commented Apr 2, 2026

Review Summary by Qodo

Walkthroughs

File Changes

Uh oh!

qodo-code-review bot commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

openshift-ci bot commented Apr 2, 2026

Uh oh!

openshift-ci bot commented Apr 2, 2026

Uh oh!

jcantrill commented Apr 6, 2026

Uh oh!

jcantrill Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

jcantrill Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

jcantrill Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

jcantrill Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

jcantrill Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

openshift-ci-robot commented Apr 2, 2026 •

edited by openshift-ci bot

Loading

qodo-code-review bot commented Apr 2, 2026 •

edited

Loading