Skip to content

Conversation

apahim
Copy link

@apahim apahim commented Aug 11, 2025

Summary

This PR adds support for cross-account CloudWatch log forwarding using AWS AssumeRole with automatically generated session names that include cluster and forwarder context.

What's Changed

  • New AssumeRole Support: Added AssumeRole field to CloudwatchIAMRole for cross-account access
  • Enhanced Security: Added optional ExternalID field for additional security measures
  • Auto-Generated Session Names: auto-generated with format: <cluster_ID>-<CLF_Name>-<output_name>. Example: 625f0e4d-cp-logs-out-to-cw
  • Dual-Profile Authentication: Implements proper authentication separation using AWS credentials profiles

Key Features

Cross-Account AssumeRole

    outputs:
      - name: out-to-cw
        type: cloudwatch
        cloudwatch:
          region: us-east-1
          groupName: app-logs
          authentication:
            type: iamRole
            iamRole:
              roleARN:
                key: role_arn
                secretName: app-logs  # Initial role for authentication
              token:
                from: serviceAccount
              assumeRole:  # NEW: Cross-account assume role
                roleARN:
                  secretName: app-logs-cross  # Cross-account role ARN
                  key: role_arn

Vector Configuration with AssumeRole

Here's the exact Vector configuration that's generated when using AssumeRole. The key
parts are:

  1. CloudWatch Sink Configuration
# Cloudwatch Logs
[sinks.output_out_to_cw]
type = "aws_cloudwatch_logs"
inputs = ["output_out_to_cw_group_name"]
region = "us-east-1"
compression = "none"
group_name = "{{ _internal.output_out_to_cw_group_name }}"
stream_name = "{{ stream_name }}"
auth.credentials_file = "/var/run/ocp-collector/config/app-logs-aws-creds/credentials"
auth.profile = "output_out-to-cw"
healthcheck.enabled = false
  1. AWS Credentials File (Referenced Above)
# Source profile for initial web identity authentication
[output_out-to-cw_source]
web_identity_token_file=/var/run/ocp-collector/serviceaccount/token
role_arn=arn:aws:iam::123456789012:role/app-logs
role_session_name=output-out-to-cw-source

# Assume role profile for cross-account access
[output_out-to-cw]
source_profile=output_out-to-cw_source
role_arn=arn:aws:iam::987654321098:role/app-logs-cross
role_session_name=625f0e4d-app-logs-out-to-cw

Key Points:

  1. Authentication Method: Vector uses credentials_file + profile approach instead of
    direct auth parameters
  2. Profile Name: output_out-to-cw (matches the output name)
  3. Dual-Profile Structure:
  • Source profile handles initial STS web identity authentication
  • Main profile handles the AssumeRole with custom session name
  1. Session Name: 625f0e4d-app-logs-out-to-cw (cluster_ID-CLF_Name-output_name)
  2. No Direct AssumeRole in Vector Config: The AssumeRole is handled entirely through the
    AWS credentials file

AWS IAM Configuration for Cross-Account Access

STS on AWS

Initial Role (Cluster Account, e.g. 123456789012:role/app-logs)

The initial role in your cluster account needs:

  1. Trust Policy to ServiceAccount (STS/OIDC):
{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Principal": {
                "Federated": "arn:aws:iam::123456789012:oidc-provider/example.com/289hcj7"
            },
            "Action": "sts:AssumeRoleWithWebIdentity",
            "Condition": {
                "StringEquals": {
                    "oidc-provider/example.com/289hcj7:sub": "system:serviceaccount:openshift-logging:app-logs"
                }
            }
        }
    ]
}
  1. AssumeRole Permissions:
{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Action": "sts:AssumeRole",
            "Resource": "arn:aws:iam::123456789012:role/app-logs"
        }
    ]
}

Target Role (Cross-account, e.g. 987654321098:role/app-logs-cross)

The target role in the external account needs:

  1. Trust Policy to source Role:
{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Principal": {
                "AWS": "arn:aws:iam::123456789012:role/app-logs"
            },
            "Action": "sts:AssumeRole",
            "Condition": {
                "StringEquals": {
                    "sts:ExternalId": "your-unique-external-id"
                }
            }
        }
    ]
}
  1. CloudWatch Permissions:
{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Action": [
                "logs:CreateLogGroup",
                "logs:CreateLogStream", 
                "logs:PutLogEvents",
                "logs:DescribeLogGroups",
                "logs:DescribeLogStreams"
            ],
            "Resource": "*"
        }
    ]
}

Backward Compatibility

No breaking changes for existing users

  • New assumeRole property is optional
  • Existing IAM Role configurations without AssumeRole continue to use output-{outputName} session names
  • New session name format only applies to new AssumeRole configurations
  • All existing CloudWatch outputs remain fully functional

Technical Details

  • Session names include cluster metadata for better traceability
  • Prevents mixed authentication modes that cause Vector parsing errors
  • Comprehensive test coverage for all scenarios

Testing Verified

  • ✅ Cross-account log forwarding working in live cluster
  • ✅ Session name format verified: 625f0e4d-cp-logs-out-to-cw
  • ✅ AWS credentials properly separated by profile
  • ✅ Backward compatibility confirmed for existing configurations
  • ✅ All unit tests passing

This resolves authentication issues in cross-account scenarios while maintaining full backward compatibility for existing CloudWatch outputs.

@openshift-ci openshift-ci bot requested review from alanconway and jcantrill August 11, 2025 14:49
Copy link
Contributor

openshift-ci bot commented Aug 11, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: apahim
Once this PR has been reviewed and has the lgtm label, please assign jcantrill for approval. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

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

Copy link
Contributor

@jcantrill jcantrill left a comment

Choose a reason for hiding this comment

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

/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 13, 2025
…sion names

Add support for cross-account CloudWatch log forwarding using
AssumeRole.

with automatically

- Add AssumeRole field to CloudwatchIAMRole for cross-account access
- Add ExternalID field for optional enhanced security
- Use AssumeRole session names generated following the pattern:
<cluster_ID>-<CLF_Name>-<output_name>
- Use dual-profile AWS credentials approach for authentication
separation

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
@apahim
Copy link
Author

apahim commented Aug 14, 2025

/retest

@cahartma
Copy link
Contributor

Looks good from review. I'm pulling down the changes today to get a closer look.

@apahim
Copy link
Author

apahim commented Aug 18, 2025

/retest

@apahim
Copy link
Author

apahim commented Sep 3, 2025

/retest

This commit addresses all review comments from @jcantrill on the
AssumeRole
PR to improve code quality, documentation, and session name generation.

Changes include:
- Update AssumeRole API documentation to be more specific about
cross-account use cases
- Remove excessive documentation sections about session names and
external ID
- Update session name format to
{clusterId}-{namespace}-{clfName}-{outputName}
- Pass cluster ID from ForwarderContext instead of dynamic retrieval
- Replace custom min function with Go's built-in min function
- Refactor ParseRoleArn and ParseAssumeRoleArn to use shared
parseAwsRoleArn function
- Update all test cases to work with new function signatures

The session name format now provides better uniqueness and traceability
for AssumeRole sessions across different namespaces and
ClusterLogForwarder
instances, improving CloudTrail auditing capabilities.

Manual testing verified that cross-account log forwarding continues to
work
correctly with the new session name format:
625f0e4d-openshift-logging-cp-logs-out-to-cw

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Copy link
Contributor

openshift-ci bot commented Sep 3, 2025

@apahim: all tests passed!

Full PR test history. Your PR dashboard.

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.

@apahim apahim requested a review from jcantrill September 3, 2025 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. release/6.4
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants