-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
Logs: Faulty Resource Policy generated #17544
Comments
I have just ran into this issue. A CDK Stack that created a log group now fails after upgrade to 1.132.0. In my case, I am creating a CodeBuild project in the stack and the IAM role assigned already has the required permissions. Looks like resource policy support was added and it is adding the policy by default (auto permissions I guess). Maybe an option is required when creating the LogGroup whether resource policy should be created automatically or not. Reverting back to 1.130.0 works for my existing stack. |
Same problem here: Reverting to 1.130.0 fixed it for me as well. The issue seems to persist on 1.133.0, so there is no point in updating. |
Same problem here but for MSK |
Unable to reproduce on CDK 2.0.0. @vramanlal @ivstiv @bjornhandersson can you share a stack that reproduces this? |
@comcalvi logGroup.grantWrite(new iam.ArnPrincipal(cluster.ref)); I guess this is no longer needed when the ResourcePolicy is "automatically" created in newer versions of CDK. Stack: import { Stack, StackProps } from 'aws-cdk-lib';
import { Vpc } from 'aws-cdk-lib/aws-ec2';
import * as logs from 'aws-cdk-lib/aws-logs';
import * as msk from 'aws-cdk-lib/aws-msk';
import * as iam from 'aws-cdk-lib/aws-iam';
import { Construct } from 'constructs';
// import * as sqs from 'aws-cdk-lib/aws-sqs';
export class LogPolicyReprodStack extends Stack {
constructor(scope: Construct, id: string, props?: StackProps) {
super(scope, id, props);
var vpc = new Vpc(this, 'SomeVpc', {
maxAzs: 2
});
const logGroup = new logs.LogGroup(this, 'BrokerLogs', {
retention: logs.RetentionDays.ONE_MONTH
});
// I'm using Cfn resource in this stack since when it was originally created no higher level construct existed.
const cluster = new msk.CfnCluster(this, 'SomeCluster', {
clusterName: 'SomeClusterV1',
kafkaVersion: '2.6.0',
numberOfBrokerNodes: 2,
loggingInfo: {
brokerLogs: {
cloudWatchLogs: {
enabled: true,
logGroup: logGroup.logGroupName
}
}
},
enhancedMonitoring: 'PER_TOPIC_PER_BROKER',
encryptionInfo: {
encryptionInTransit: {
clientBroker: 'TLS_PLAINTEXT',
inCluster: false
}
},
brokerNodeGroupInfo: {
clientSubnets: vpc.privateSubnets.map(subnet => subnet.subnetId),
instanceType: 'kafka.t3.small',
storageInfo: {
ebsStorageInfo: {
volumeSize: 8
}
}
}
});
// This is the line that causes the problem.
logGroup.grantWrite(new iam.ArnPrincipal(cluster.ref));
}
} |
@bjornhandersson thanks for the stack, I can confirm that it reproduces the issue. However the line that you rightfully pointed out causes the problem:
is working as intended. The This reproduces the error for the wrong reason (the error is desired here). Did this successfully deploy on a version prior to 1.132.0? Can someone provide a stack that successfully deployed on older versions but was failed deployment with that error message when you upgraded to 1.132.0? |
Thanks for the reply. |
I did a few runs with 1.133.0 & 1.136.0 stack dependencies (npm) and could not replicate its previous behaviour from a few weeks ago when I consistently experienced the error and had to roll back the update of the dependencies. I have also updated my aws-cdk npm package since then which might also be related somehow. I guess the issue has been resolved?! Unfortunately (or perhaps fortunately 👍 ) I can't help with any broken examples at this point of time. |
@bjornhandersson I can confirm that it did successfully deploy on version 1.98.0. However, if the stack is first deployed without
and then that line is added and you run @ivstiv thanks for the reply. Glad to hear that the issue has been resolved. |
The following code with comments show when it works and when it does not. I appears to be related to Fn.importValue when importing a role via ARN created in another stack. Also if the stack props do not provide the env values, then the Fn.importValue lines work. That is no LogResourcPolicy is output. This code has been tested with CDK 1.1.36.0.
|
Hello there. I figure out with that issue just let CDK create the IAM ExecutionRole + IAM policy for ECS service byself. |
I also have this issue when trying to create a log group and use it for an ECS Task container with version 1.139.0. |
having a same issue on v1.135.0 |
@nomadme @FrontierPsychiatrist @Stazz-Sphinx @vramanlal How is the exported value generated? Is it from a CDK stack, CloudFormation only stack, or a resource created via the console / SDKs? |
update on the original bug: turns out I did a rookie mistake and didn't set the appropriate environment in |
@comcalvi I am not sure what you mean by "exported value". I am using a CDK stack (in Typescript if that's relevant). Today I upgraded from 1.141.0 to 2.10.0 and still get the same error when I try to deploy the stack.
I am creating a log group manually and then attaching it to an ECS task container const logGroup = new logs.LogGroup(this, 'LogGroup', {
logGroupName: '/ecs/someName',
retention: logs.RetentionDays.ONE_MONTH,
removalPolicy: RemovalPolicy.DESTROY,
})
task.addContainer('Reporter', {
image: ecs.RepositoryImage.fromEcrRepository(repository, imageTag.valueAsString),
environment: {
// ...
},
logging: ecs.AwsLogDriver.awsLogs({
streamPrefix: '',
logGroup: logGroup
}),
secrets: {
// ...
}
}) The task is allowed to create log streams via a shared execution role that I load via it's ARN (but with I initially only had the ECR repository in the stack and successfully deployed it, but now after adding the other resources deployments always fail. Even with multiple |
@comcalvi Yes, from CDK. |
@comcalvi yes the exported values "devops-kms-key-codebuild-arn" and "devops-kms-key-logs-arn" are in another CDK stack. Interesting note is that if in the StackProps is DO NOT provide the env object (account and region), the stack deploys as expected. However when I provide the env object - then it fails. The exported values created by the other stack as also in the same account and region. |
@comcalvi - On digging through Ctrail, this is what I find - "Principal section of policy contains ARN instead of account ID: arn:aws:iam::8428xxx:role/BlueGreenContainerImageSt-EcsBlueGreenRolesecsTask-1ETUWEFPQ9D7L", |
@comcalvi So our deployment is returning with this msg. It seems like the underlying cloudformation template is creating a malformed request.
Please help. |
@rix0rrr You closed this ticket, but you didn't provide clear direction of how to remediate this error. WHAT IS THE SOLUTION TO RESOLVE THIS ERROR? |
The other ticket was closed because it is the same issue as this one. We are working on a fix for this now. There is currently no known workaround to this bug without downgrading to one of the versions mentioned above that had successful deployments. |
Thanks @comcalvi for the status update. Do you have any ETA when the patch will be released? If this is going to take a while, we can rollback to previous version. |
likely 1-3 weeks. |
This is a possible workaround for logging issue const logGroup = new LogGroup(this, `${API_PREFIX}LogGroup`, {
... some props
});
const taskDefinition = new FargateTaskDefinition(this, `${API_PREFIX}TaskDefinition`, {
... some props
});
taskDefinition.addContainer(`${API_PREFIX}Container`, {
... some props
// -- Disabled logging in container, this is where the bug in CDK lives.
// logging: LogDrivers.awsLogs({
// streamPrefix: API_PREFIX,
// logGroup: logGroup
// })
}
);
// -- Logging is added with a cfnTaskDefinition override
const cfnTaskDefinition = taskDefinition.node.defaultChild as CfnTaskDefinition;
cfnTaskDefinition.addOverride("Properties.ContainerDefinitions.0.LogConfiguration", {
"LogDriver": "awslogs",
"Options": {
"awslogs-group": (logGroup.node.defaultChild as CfnLogGroup).ref,
"awslogs-stream-prefix": API_PREFIX,
"awslogs-region": "*****" // change this to actual region
}
}); |
@BuruY Thanks, using the level 1 class works. |
Thanks @BuruY !
|
Any update on this? The 1 to 3 weeks are over since some time. Still not working on my side. Also on 1.147 the same issue happens. |
This is taking too long. Can someone provide a L1 example fix using Python code, please :-) |
FWIW this issue seems to be fixed in the latest Python CDK version 2.17.0 |
Not fixed in Typescript CDK Version 2.17.0. Still facing the same issue. |
Not for us. Started seeing this after moving a log group for ECS task to a different construct (new resource ID). Have tried to upgrade from 2.15.0 to 2.17.0, and it didn't fix it. |
Closes #17544. Cloudwatch logs resource policies do not accept ARNs of any kind as principals. This PR adds logic to convert any ARN principals to account ID strings for resource policies and provides methods to do so if needed in other modules, even if those ARNs are encoded as tokens (for example, if using an imported value to retrieve an ARN principal). Shout-out to @rix0rrr for coauthoring this. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
|
See [CHANGELOG](https://github.com/aws/aws-cdk/blob/bump/1.152.0/CHANGELOG.md) For convenience, extracted the relevant CHANGELOG entry: ## [1.152.0](v1.151.0...v1.152.0) (2022-04-06) ### Features * **cfnspec:** cloudformation spec v63.0.0 ([#19679](#19679)) ([dba96a9](dba96a9)) * **cfnspec:** cloudformation spec v65.0.0 ([#19745](#19745)) ([796fc64](796fc64)) * **cli:** add --build option ([#19663](#19663)) ([eb9b8e2](eb9b8e2)), closes [#19667](#19667) * **cli:** preview of `cdk import` ([#17666](#17666)) ([4f12209](4f12209)) * **core:** throw error when stack name exceeds max length ([#19725](#19725)) ([1ffd45e](1ffd45e)) * **eks:** add k8s v1.22 ([#19756](#19756)) ([9a518c5](9a518c5)) * **opensearch:** Add latest Opensearch Version 1.2 ([#19749](#19749)) ([a2ac36e](a2ac36e)) * add new integration test runner ([#19754](#19754)) ([1b4d010](1b4d010)) * **eks:** alb-controller v2.4.1 ([#19653](#19653)) ([1ec08df](1ec08df)) * **lambda:** add support for ephemeral storage ([#19552](#19552)) ([f1d9b6a](f1d9b6a)), closes [#19605](#19605) * **s3:** EventBridge bucket notifications ([#18614](#18614)) ([d8e602b](d8e602b)), closes [#18076](#18076) * **synthetics:** new puppeteer 3.5 runtime ([#19673](#19673)) ([ce2b91b](ce2b91b)), closes [#19634](#19634) ### Bug Fixes * **aws_applicationautoscaling:** Add missing members to PredefinedMetric enum ([#18978](#18978)) ([75a6fa7](75a6fa7)), closes [#18969](#18969) * **cli:** apps with many resources scroll resource output offscreen ([#19742](#19742)) ([053d22c](053d22c)), closes [#19160](#19160) * **cli:** support attributes of DynamoDB Tables for hotswapping ([#19620](#19620)) ([2321ece](2321ece)), closes [#19421](#19421) * **cloudwatch:** automatic metric math label cannot be suppressed ([#17639](#17639)) ([7fa3bf2](7fa3bf2)) * **codedeploy:** add name validation for Application, Deployment Group and Deployment Configuration ([#19473](#19473)) ([9185042](9185042)) * **codedeploy:** the Service Principal is wrong in isolated regions ([#19729](#19729)) ([7e9a43d](7e9a43d)), closes [#19399](#19399) * **core:** `Fn.select` incorrectly short-circuits complex expressions ([#19680](#19680)) ([7f26fad](7f26fad)) * **core:** detect and resolve stringified number tokens ([#19578](#19578)) ([7d9ab2a](7d9ab2a)), closes [#19546](#19546) [#19550](#19550) * **core:** reduce CFN template indent size to save bytes ([#19656](#19656)) ([fd63ca3](fd63ca3)) * **ecs:** 'desiredCount' and 'ephemeralStorageGiB' cannot be tokens ([#19453](#19453)) ([c852239](c852239)), closes [#16648](#16648) * **ecs:** remove unnecessary error when adding volume to external task definition ([#19774](#19774)) ([5446ded](5446ded)), closes [#19259](#19259) * **iam:** policies aren't minimized as far as possible ([#19764](#19764)) ([876ed8a](876ed8a)), closes [#19751](#19751) * **logs:** Faulty Resource Policy Generated ([#19640](#19640)) ([1fdf122](1fdf122)), closes [#17544](#17544)
See [CHANGELOG](https://github.com/aws/aws-cdk/blob/bump/2.20.0/CHANGELOG.md) For convenience, extracted the relevant CHANGELOG entry: ## [2.20.0](v2.19.0...v2.20.0) (2022-04-07) ### Features * **cfnspec:** cloudformation spec v63.0.0 ([#19679](#19679)) ([dba96a9](dba96a9)) * **cfnspec:** cloudformation spec v65.0.0 ([#19745](#19745)) ([796fc64](796fc64)) * **cli:** add --build option ([#19663](#19663)) ([eb9b8e2](eb9b8e2)), closes [#19667](#19667) * **cli:** preview of `cdk import` ([#17666](#17666)) ([4f12209](4f12209)) * **core:** throw error when stack name exceeds max length ([#19725](#19725)) ([1ffd45e](1ffd45e)) * **eks:** add k8s v1.22 ([#19756](#19756)) ([9a518c5](9a518c5)) * **opensearch:** Add latest Opensearch Version 1.2 ([#19749](#19749)) ([a2ac36e](a2ac36e)) * add new integration test runner ([#19754](#19754)) ([1b4d010](1b4d010)) * **eks:** alb-controller v2.4.1 ([#19653](#19653)) ([1ec08df](1ec08df)) * **lambda:** add support for ephemeral storage ([#19552](#19552)) ([f1d9b6a](f1d9b6a)), closes [#19605](#19605) * **s3:** EventBridge bucket notifications ([#18614](#18614)) ([d8e602b](d8e602b)), closes [#18076](#18076) ### Bug Fixes * **aws_applicationautoscaling:** Add missing members to PredefinedMetric enum ([#18978](#18978)) ([75a6fa7](75a6fa7)), closes [#18969](#18969) * **cli:** apps with many resources scroll resource output offscreen ([#19742](#19742)) ([053d22c](053d22c)), closes [#19160](#19160) * **cli:** support attributes of DynamoDB Tables for hotswapping ([#19620](#19620)) ([2321ece](2321ece)), closes [#19421](#19421) * **cloudwatch:** automatic metric math label cannot be suppressed ([#17639](#17639)) ([7fa3bf2](7fa3bf2)) * **codedeploy:** add name validation for Application, Deployment Group and Deployment Configuration ([#19473](#19473)) ([9185042](9185042)) * **codedeploy:** the Service Principal is wrong in isolated regions ([#19729](#19729)) ([7e9a43d](7e9a43d)), closes [#19399](#19399) * **core:** `Fn.select` incorrectly short-circuits complex expressions ([#19680](#19680)) ([7f26fad](7f26fad)) * **core:** detect and resolve stringified number tokens ([#19578](#19578)) ([7d9ab2a](7d9ab2a)), closes [#19546](#19546) [#19550](#19550) * **core:** reduce CFN template indent size to save bytes ([#19656](#19656)) ([fd63ca3](fd63ca3)) * **ecs:** 'desiredCount' and 'ephemeralStorageGiB' cannot be tokens ([#19453](#19453)) ([c852239](c852239)), closes [#16648](#16648) * **ecs:** remove unnecessary error when adding volume to external task definition ([#19774](#19774)) ([5446ded](5446ded)), closes [#19259](#19259) * **iam:** policies aren't minimized as far as possible ([#19764](#19764)) ([876ed8a](876ed8a)), closes [#19751](#19751) * **logs:** Faulty Resource Policy Generated ([#19640](#19640)) ([1fdf122](1fdf122)), closes [#17544](#17544)
Closes aws#17544. Cloudwatch logs resource policies do not accept ARNs of any kind as principals. This PR adds logic to convert any ARN principals to account ID strings for resource policies and provides methods to do so if needed in other modules, even if those ARNs are encoded as tokens (for example, if using an imported value to retrieve an ARN principal). Shout-out to @rix0rrr for coauthoring this. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
What is the problem?
I'm attempting to create a simple stack that houses a task definition for some fargate tasks.
When I add a logging configuration to a container definition inside, a resource policy is added in the generated cf template:
Reproduction Steps
What did you expect to happen?
I was attempting to generate a template that behaved similarly to a CF template I've written below.
When deployed, a task definition with a single container definition is created, with references to the pre-existing resources I've specified using ARNs.
What actually happened?
CDK CLI Version
1.132.0 (build 5c75891)
Framework Version
No response
Node.js Version
v12.21.0
OS
MacOS Catalina 10.15.7
Language
Python
Language Version
3.9.6
Other information
Our AWS account is controlled by a information security service provider - I thought that perhaps this was a personal account permissions issue. However, the error is
InvalidRequest
and notAccessDenied
so I'm gut checking here first.The text was updated successfully, but these errors were encountered: