LOG-6790: Update cw forwarding to use credential file and profiles #3008

Clee2691 · 2025-04-04T19:25:36Z

Description

This PR adds the ability for the ClusterLogForwarder to utilize multiple distinct role_arns to authenticate to amazon's Cloudwatch on an STS enabled openshift cluster.

Changes:

Removes validation for multiple role_arns.
Adds validation to ensure valid role_arns.
Creates and reconciles a credentials file as a configMap with multiple profiles mounted in /var/run/ocp-collector/config/<CONFIGMAP NAME>/credentials.
The cloudwatch sink will specify the credentials file along with the profile name when iamRole is used as authentication.
No longer sets environment variables for cloudwatch.
Added documentation for an example workflow.

This PR depends on changes to the RH Vector build to include a fix for the region when authenticating to AWS.

/cc @cahartma @vparfonov
/assign @jcantrill

Links

Depending on PR(s): LOG-6789: Missing region error when using AWS credentials file ViaQ/vector#197
JIRA: https://issues.redhat.com/browse/LOG-6790

openshift-ci-robot · 2025-04-04T19:25:40Z

@Clee2691: This pull request references LOG-6790 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

This PR adds the ability for the ClusterLogForwarder to utilize multiple distinct role_arns to authenticate to amazon's Cloudwatch on an STS enabled openshift cluster.

Changes:

Removes validation for multiple role_arns.

Adds validation to ensure valid role_arns.

Creates and reconciles a credentials file as a configMap with multiple profiles mounted in /var/run/ocp-collector/config/<CONFIGMAP NAME>/credentials.

The cloudwatch sink will specify the credentials file along with the profile name when iamRole is used as authentication.

No longer sets environment variables for cloudwatch.

Added documentation for an example workflow.

This PR depends on changes to the RH Vector build to include a fix for the when authenticating to AWS.

/cc @cahartma @vparfonov
/assign @jcantrill

Links

Depending on PR(s): LOG-6789: Missing region error when using AWS credentials file ViaQ/vector#197

JIRA: https://issues.redhat.com/browse/LOG-6790

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.

openshift-ci · 2025-04-04T19:25:57Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Clee2691 · 2025-04-04T19:26:06Z

/test all

openshift-ci-robot · 2025-04-07T14:17:32Z

@Clee2691: This pull request references LOG-6790 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

This PR adds the ability for the ClusterLogForwarder to utilize multiple distinct role_arns to authenticate to amazon's Cloudwatch on an STS enabled openshift cluster.

Changes:

Removes validation for multiple role_arns.

Adds validation to ensure valid role_arns.

Creates and reconciles a credentials file as a configMap with multiple profiles mounted in /var/run/ocp-collector/config/<CONFIGMAP NAME>/credentials.

The cloudwatch sink will specify the credentials file along with the profile name when iamRole is used as authentication.

No longer sets environment variables for cloudwatch.

Added documentation for an example workflow.

This PR depends on changes to the RH Vector build to include a fix for the region when authenticating to AWS.

/cc @cahartma @vparfonov
/assign @jcantrill

Links

Depending on PR(s): LOG-6789: Missing region error when using AWS credentials file ViaQ/vector#197

JIRA: https://issues.redhat.com/browse/LOG-6790

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.

internal/collector/cloudwatch.credentials.tmpl

internal/collector/cloudwatch.go

jcantrill · 2025-04-07T20:10:06Z

internal/collector/cloudwatch.go

+}
+
+// ReconcileAWSCredentialsConfigMap reconciles a configmap with credential profile(s) for Cloudwatch output(s).
+func ReconcileAWSCredentialsConfigMap(k8sClient client.Client, reader client.Reader, namespace, name string, outputs []obs.OutputSpec, secrets observability.Secrets, configMaps map[string]*corev1.ConfigMap, owner metav1.OwnerReference) (*corev1.ConfigMap, error) {


Do we need the uncached reader other then k8sClient here? Usually that is only an issue when trying to read a cluster resource that we don't own

Well the reader is used in reconcile.configmap probably because it is used for reconciling the dashboard configmap. So yes

The dashboard, however, lives in an unmanaged namespace. You should not need to pass the reader here and simply pass the client as the reader in the dependent function

I see what you mean. Will make the change

internal/constants/constants.go

jcantrill · 2025-04-07T20:16:48Z

internal/controller/observability/collector.go

+
+	// Add generated credentials configmap to contexts to be mounted in pod
+	if credCm != nil {
+		context.ConfigMaps[credCm.Name] = credCm


Chance of collision wiht a spec'd secret name. We may need an early validation to ensure there is none

We create other configmaps without name collision validation. I've already generated the name with the forwarder name + aws-creds as that follows our current resource name conventions. I could implement early validation but then should we validate other resource names?

If we have precedence and are certain there is limited chance of collision then leave it as-is

I think we will have precedence because it will overwrite what is spec'd when the data etc does not match

internal/generator/vector/output/cloudwatch/cloudwatch.go

internal/generator/vector/conf/conf.go

Clee2691 · 2025-04-08T15:43:39Z

/test all

Clee2691 · 2025-04-08T18:25:06Z

/test unit

Clee2691 · 2025-04-09T17:42:30Z

/hold

jcantrill · 2025-04-09T18:00:35Z

/approve

openshift-ci · 2025-04-09T18:01:08Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Clee2691, jcantrill

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~~ [jcantrill]

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

Clee2691 · 2025-04-09T22:56:04Z

/retest

Clee2691 · 2025-04-10T13:16:25Z

/retest

Clee2691 · 2025-04-10T17:32:32Z

/retest

Clee2691 · 2025-04-10T20:42:26Z

/retest

Clee2691 · 2025-04-11T15:32:30Z

/retest

Clee2691 · 2025-04-11T20:34:05Z

/retest

Clee2691 · 2025-04-18T13:50:51Z

/hold cancel

cahartma · 2025-04-18T14:07:06Z

/lgtm

openshift-ci-robot · 2025-04-18T15:22:32Z

/retest-required

Remaining retests: 0 against base HEAD ad4c79f and 2 for PR HEAD b927d68 in total

openshift-ci · 2025-04-18T17:03:42Z

@Clee2691: 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.

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

openshift-ci bot assigned jcantrill Apr 4, 2025

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 4, 2025

openshift-ci bot requested review from cahartma and vparfonov April 4, 2025 19:25

jcantrill added the release/6.3 label Apr 7, 2025

jcantrill reviewed Apr 7, 2025

View reviewed changes

Clee2691 force-pushed the LOG-6790 branch from c1f328e to 901c622 Compare April 8, 2025 15:42

Clee2691 marked this pull request as ready for review April 9, 2025 17:42

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 9, 2025

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 9, 2025

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

Clee2691 force-pushed the LOG-6790 branch from 901c622 to 49c07d4 Compare April 9, 2025 18:54

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 11, 2025

Clee2691 force-pushed the LOG-6790 branch from 49c07d4 to 9ec583e Compare April 14, 2025 17:57

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 14, 2025

LOG-6790: Update cw forwarding to use credential file and profiles

b927d68

Clee2691 force-pushed the LOG-6790 branch from 9ec583e to b927d68 Compare April 14, 2025 18:28

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

openshift-ci bot assigned cahartma Apr 18, 2025

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 18, 2025

openshift-merge-bot bot merged commit dbeb897 into openshift:master Apr 18, 2025
8 checks passed

LOG-6790: Update cw forwarding to use credential file and profiles #3008

LOG-6790: Update cw forwarding to use credential file and profiles #3008

Uh oh!

Conversation

Clee2691 commented Apr 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Links

Uh oh!

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

Uh oh!

Description

Links

Uh oh!

openshift-ci bot commented Apr 4, 2025

Uh oh!

Clee2691 commented Apr 4, 2025

Uh oh!

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

Uh oh!

Description

Links

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jcantrill Apr 7, 2025

Choose a reason for hiding this comment

Uh oh!

Clee2691 Apr 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcantrill Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

Clee2691 Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jcantrill Apr 7, 2025

Choose a reason for hiding this comment

Uh oh!

Clee2691 Apr 8, 2025

Choose a reason for hiding this comment

Uh oh!

jcantrill Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

Clee2691 Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Clee2691 commented Apr 8, 2025

Uh oh!

Clee2691 commented Apr 8, 2025

Uh oh!

Clee2691 commented Apr 9, 2025

Uh oh!

jcantrill commented Apr 9, 2025

Uh oh!

openshift-ci bot commented Apr 9, 2025

Uh oh!

Clee2691 commented Apr 9, 2025

Uh oh!

Clee2691 commented Apr 10, 2025

Uh oh!

Clee2691 commented Apr 10, 2025

Uh oh!

Clee2691 commented Apr 10, 2025

Uh oh!

Clee2691 commented Apr 11, 2025

Uh oh!

Clee2691 commented Apr 11, 2025

Uh oh!

Clee2691 commented Apr 18, 2025

Uh oh!

cahartma commented Apr 18, 2025

Uh oh!

Clee2691 commented Apr 4, 2025 •

edited

Loading

openshift-ci-robot commented Apr 4, 2025 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Apr 7, 2025 •

edited by openshift-ci bot

Loading

Clee2691 Apr 8, 2025 •

edited

Loading