Skip to content
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

elasticloadbalancing: allow logAccessLogs on environment agnostic stack #27432

Open
1 of 2 tasks
Mahoney opened this issue Oct 6, 2023 · 4 comments
Open
1 of 2 tasks
Labels
@aws-cdk/aws-elasticloadbalancing Related to Amazon Elastic Load Balancing effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p2

Comments

@Mahoney
Copy link

Mahoney commented Oct 6, 2023

Describe the feature

Currently BaseLoadBalancer.logAccessLogs requires that the Stack has a specific region specified on the Environment.

This seems a pretty arbitrary limitation - why should this be necessary? Looking at the code, it seems to be in order to set a principal, but other logging constructs like flowLog allow specifying roles, and indeed if the region doesn't resolve to an account in BaseLoadBalancer.resourcePolicyPrincipal it just returns iam.ServicePrincipal('logdelivery.elasticloadbalancing.amazonaws.com'), so why shouldn't it do that if the region is unresolved too, as it's only using the region to find an account?

Use Case

I want to enable access logging on an ALB created in an environment agnostic stack, which seems a reasonable thing to do.

Proposed Solution

Change https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/shared/base-load-balancer.ts#L306-L309
as so:

    const region = Stack.of(this).region;
    if (Token.isUnresolved(region)) {
      return new iam.ServicePrincipal('logdelivery.elasticloadbalancing.amazonaws.com');
    }

Other Information

Context - we can't set a region. In our case we have a CDK setup that has been running fine for over a year. A PEN test has flagged that we should have access logs for our ALBs. We have a Product Stack as part of our Stack, and setting a region on the Environment for the Stack completely breaks it; if I set just a region I get this:

Error: Stack "my-stack/my-product" cannot reference {my-stack/my-stack-vpc/publicSubnet1/Subnet[Ref]} in stack "my-stack". Cross stack references are only supported for stacks deployed to the same environment or between nested stacks and their parent stack. Set crossRegionReferences=true to enable cross region references

(I am setting crossRegionReferences(true))

If I set a region and an account I get this:

Resolution error: Cannot generate a physical name for my-stack/my-product/my-stack-my-product-ecs-service-task-def/ExecutionRole, because the region is un-resolved or missing.

There seems no way to specify an environment on a product stack.

So currently I'm completely stymied. I guess I'll just have to set up the access logging manually as a handcrafted little snowflake.

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.99.1

Environment details (OS name and version, etc.)

macOs & linux

@Mahoney Mahoney added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Oct 6, 2023
@github-actions github-actions bot added the @aws-cdk/aws-elasticloadbalancing Related to Amazon Elastic Load Balancing label Oct 6, 2023
@peterwoodworth peterwoodworth added good first issue Related to contributions. See CONTRIBUTING.md p1 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Oct 6, 2023
@peterwoodworth
Copy link
Contributor

Thanks for the detailed report. It makes sense, I don't see why your proposed solution wouldn't work

@tmokmss
Copy link
Contributor

tmokmss commented Nov 15, 2023

@Mahoney
According to the doc, the elb-account-id is necessary for older regions, so we cannot easily synthesize env-agnostic templates with elb access logging. See this comment for more detail: #27938 (comment)

I think the problem we need to really solve is the below; how the region is missing where you specified it explicitly?

Resolution error: Cannot generate a physical name for my-stack/my-product/my-stack-my-product-ecs-service-task-def/ExecutionRole, because the region is un-resolved or missing.

@pahud pahud added p2 and removed p1 labels Jun 11, 2024
@CaptnMichi
Copy link

We're facing the same issue here. I want to enable access logging on a ALB in us-east-2 with a bucket in us-east-2. When i set the region on stack level, other stacks that inherit this stack will break with

Error: Stack "<stack>" cannot reference {<stack-name>/nginxDeliveryStream/firehoseDeliveryStream[Arn]} in stack "<another-stack>". Cross stack references are only supported for stacks deployed to the same environment or between nested stacks and their parent stack. Set crossRegionReferences=true to enable cross region references

aws-cdk-lib": "^2.150.0

@bpottier
Copy link

bpottier commented Dec 4, 2024

We're seeing the same issue with TaskDefinition/ExecutionRole. Is there a workaround at least?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-elasticloadbalancing Related to Amazon Elastic Load Balancing effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p2
Projects
None yet
6 participants