aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.34k stars 3.76k forks source link

aws-appconfig: SourcedConfiguration doesn't use retrievalRole #30609

Open maikbasel opened 1 week ago

maikbasel commented 1 week ago

Describe the bug

The SourcedConfiguration construct does not make use of it's retrievalRole property regardless of wether is defined or not. It will always create a new IAM Role (except when it's location type is ConfigurationSourceType.CODE_PIPELINE then it will default to undefined).

I encountered this behavior while trying to use role of mine created in a different stack and encountered an error that the role used was not allowed to retrieve the secret my configuration profile should reference.

Expected Behavior

The SourcedConfiguration should make use of the role passed to it via the retrievalRole property.

Current Behavior

The SourcedConfiguration will always create a new IAM Role instead of using the role passed into it via the retrievalRole property.

Reproduction Steps

Here a simplyfied version of my code:

StackA:

import {Stack, StackProps} from "aws-cdk-lib";
import {Construct} from "constructs";
import {Role, ServicePrincipal} from "aws-cdk-lib/aws-iam";
import {Secret} from "aws-cdk-lib/aws-secretsmanager";

export class StackA extends Stack {

    constructor(scope: Construct, id: string, props: StackProps) {
        super(scope, id, props);

        const appConfigRetrievalRole = new Role(this, 'AppConfigRetrieveRole', {
            assumedBy: new ServicePrincipal('appconfig.amazonaws.com'),
            roleName: 'AppConfigRetrieveRole',
        });

        const mySecret = new Secret(this, 'MySecret', {
            secretName: 'MySecret',
            generateSecretString: {
                secretStringTemplate: JSON.stringify({
                    username: 'admin',
                    password: '<PASSWORD>'
                }),
                generateStringKey: 'password'
            }
        });

        mySecret.grantRead(appConfigRetrievalRole);
    }
}

StackB:

import {Construct} from "constructs";
import {Stack, StackProps} from "aws-cdk-lib";
import {Secret} from "aws-cdk-lib/aws-secretsmanager";
import {Application, ConfigurationSource, Environment, SourcedConfiguration} from "aws-cdk-lib/aws-appconfig";
import {Role} from "aws-cdk-lib/aws-iam";

export class StackB extends Stack {
    constructor(scope: Construct, id: string, props: StackProps) {
        super(scope, id, props);

        const myAppConfigApp = new Application(this, 'MyAppConfigApp', {
            applicationName: 'my-app',
        })

        const devEnv = new Environment(this, 'DevEnv', {
            application: myAppConfigApp,
            environmentName: 'dev',
        });

        const mySecret = Secret.fromSecretNameV2(this, 'MySecret', 'MySecret');

        const appConfigRetrievalRole = Role.fromRoleName(this, 'AppConfigRetrieveRole', 'AppConfigRetrieveRole');

        new SourcedConfiguration(this, 'MySecretConfig', {
            name: 'mySecretConfig',
            application: myAppConfigApp,
            location: ConfigurationSource.fromSecret(mySecret),
            retrievalRole: appConfigRetrievalRole,
        });
    }
}

Possible Solution

The condition in the SourcedConfigurations constructor checking wether the retrievalRole is defined is wrong. As I already wrote, regardless of wether retrievalRole is defined or not. It will always create a new IAM Role (except when it's location type is ConfigurationSourceType.CODE_PIPELINE then it will default to undefined). This makes no sense. If the retrievalRole is defined it should use this role.

Additional Information/Context

No response

CDK CLI Version

2.146.0

Framework Version

No response

Node.js Version

20.14.0

OS

Windows 10

Language

TypeScript

Language Version

No response

Other information

No response

ashishdhingra commented 1 week ago

@maikbasel Good afternoon. Thanks for opening the issue. Could you please share minimal self-contained code to troubleshoot the issue? This would ensure quick reproduction and mimic the same scenario.

Thanks, Ashish

github-actions[bot] commented 1 week ago

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

maikbasel commented 1 week ago

@ashishdhingra I updated the issue with a simplyfied code example.