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

refactor(iam): cleanup of IAM library #2823

Merged
merged 15 commits into from
Jun 17, 2019
Next Next commit
refactor(iam): cleanup of IAM library
Various changes.

BREAKING CHANGE:

* **iam**: rename `ImportedResourcePrincipal` to `UnknownPrincipal`.
* **iam**: `managedPolicyArns` renamed to `managedPolicies`, takes
  return value from `ManagedPolicy.fromAwsManagedPolicyName()`.
  • Loading branch information
rix0rrr committed Jun 11, 2019
commit 4e43f0508f8bb7deba608248afa37029d9cfab82
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-codebuild/lib/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,7 @@ export class Project extends ProjectBase {

constructor(s: Construct, i: string) {
super(s, i);
this.grantPrincipal = new iam.ImportedResourcePrincipal({ resource: this });
this.grantPrincipal = new iam.UnknownPrincipal({ resource: this });
}
}

Expand Down Expand Up @@ -584,7 +584,7 @@ export class Project extends ProjectBase {
resourceName: projectName,
});

this.grantPrincipal = new iam.ImportedResourcePrincipal({ resource: this });
this.grantPrincipal = new iam.UnknownPrincipal({ resource: this });
this.projectName = projectName;
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-config/lib/managed-rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ export class CloudFormationStackDriftDetectionCheck extends ManagedRule {

this.role = props.role || new iam.Role(this, 'Role', {
assumedBy: new iam.ServicePrincipal('config.amazonaws.com'),
managedPolicyArns: [
new iam.AwsManagedPolicy('ReadOnlyAccess', this).policyArn,
managedPolicies: [
iam.ManagedPolicy.fromAwsManagedPolicyName('ReadOnlyAccess')
]
});
}
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-config/lib/rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -345,8 +345,8 @@ export class CustomRule extends RuleNew {
});

if (props.lambdaFunction.role) {
props.lambdaFunction.role.attachManagedPolicy(
new iam.AwsManagedPolicy('service-role/AWSConfigRulesExecutionRole', this).policyArn
props.lambdaFunction.role.addManagedPolicy(
iam.ManagedPolicy.fromAwsManagedPolicyName('service-role/AWSConfigRulesExecutionRole')
);
}

Expand Down
12 changes: 6 additions & 6 deletions packages/@aws-cdk/aws-eks/lib/cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,9 +208,9 @@ export class Cluster extends Resource implements ICluster {

this.role = props.role || new iam.Role(this, 'ClusterRole', {
assumedBy: new iam.ServicePrincipal('eks.amazonaws.com'),
managedPolicyArns: [
new iam.AwsManagedPolicy('AmazonEKSClusterPolicy', this).policyArn,
new iam.AwsManagedPolicy('AmazonEKSServicePolicy', this).policyArn,
managedPolicies: [
iam.ManagedPolicy.fromAwsManagedPolicyName('AmazonEKSClusterPolicy'),
iam.ManagedPolicy.fromAwsManagedPolicyName('AmazonEKSServicePolicy'),
],
});

Expand Down Expand Up @@ -308,9 +308,9 @@ export class Cluster extends Resource implements ICluster {
// FIXME: Add a cfn-signal call once we've sorted out UserData and can write reliable
// signaling scripts: https://github.com/awslabs/aws-cdk/issues/623

autoScalingGroup.role.attachManagedPolicy(new iam.AwsManagedPolicy('AmazonEKSWorkerNodePolicy', this).policyArn);
autoScalingGroup.role.attachManagedPolicy(new iam.AwsManagedPolicy('AmazonEKS_CNI_Policy', this).policyArn);
autoScalingGroup.role.attachManagedPolicy(new iam.AwsManagedPolicy('AmazonEC2ContainerRegistryReadOnly', this).policyArn);
autoScalingGroup.role.addManagedPolicy(iam.ManagedPolicy.fromAwsManagedPolicyName('AmazonEKSWorkerNodePolicy'));
autoScalingGroup.role.addManagedPolicy(iam.ManagedPolicy.fromAwsManagedPolicyName('AmazonEKS_CNI_Policy'));
autoScalingGroup.role.addManagedPolicy(iam.ManagedPolicy.fromAwsManagedPolicyName('AmazonEC2ContainerRegistryReadOnly'));

// EKS Required Tags
autoScalingGroup.node.applyAspect(new Tag(`kubernetes.io/cluster/${this.clusterName}`, 'owned', { applyToLaunchedInstances: true }));
Expand Down
19 changes: 10 additions & 9 deletions packages/@aws-cdk/aws-iam/lib/group.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { Construct, Resource, Stack } from '@aws-cdk/cdk';
import { Construct, Lazy, Resource, Stack } from '@aws-cdk/cdk';
import { CfnGroup } from './iam.generated';
import { IIdentity } from './identity-base';
import { IManagedPolicy } from './managed-policy';
import { Policy } from './policy';
import { PolicyStatement } from './policy-document';
import { ArnPrincipal, IPrincipal, PrincipalPolicyFragment } from './principals';
import { IUser } from './user';
import { AttachedPolicies, undefinedIfEmpty } from './util';
import { AttachedPolicies } from './util';

export interface IGroup extends IIdentity {
/**
Expand Down Expand Up @@ -74,7 +75,7 @@ abstract class GroupBase extends Resource implements IGroup {
policy.attachToGroup(this);
}

public attachManagedPolicy(_arn: string) {
public addManagedPolicy(_policy: IManagedPolicy) {
// drop
}

Expand Down Expand Up @@ -118,16 +119,16 @@ export class Group extends GroupBase {
public readonly groupName: string;
public readonly groupArn: string;

private readonly managedPolicies: string[];
private readonly managedPolicies: IManagedPolicy[] = [];

constructor(scope: Construct, id: string, props: GroupProps = {}) {
super(scope, id);

this.managedPolicies = props.managedPolicyArns || [];
this.managedPolicies.push(...props.managedPolicyArns || []);

const group = new CfnGroup(this, 'Resource', {
groupName: props.groupName,
managedPolicyArns: undefinedIfEmpty(() => this.managedPolicies),
managedPolicyArns: Lazy.listValue({ produce: () => this.managedPolicies.map(p => p.managedPolicyArn) }, { omitEmpty: true }),
path: props.path,
});

Expand All @@ -137,9 +138,9 @@ export class Group extends GroupBase {

/**
* Attaches a managed policy to this group.
* @param arn The ARN of the managed policy to attach.
* @param policy The managed policy to attach.
*/
public attachManagedPolicy(arn: string) {
this.managedPolicies.push(arn);
public addManagedPolicy(policy: IManagedPolicy) {
this.managedPolicies.push(policy);
}
}
5 changes: 3 additions & 2 deletions packages/@aws-cdk/aws-iam/lib/identity-base.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { IResource } from '@aws-cdk/cdk';
import { IManagedPolicy } from './managed-policy';
import { Policy } from "./policy";
import { IPrincipal } from "./principals";

Expand All @@ -15,7 +16,7 @@ export interface IIdentity extends IPrincipal, IResource {

/**
* Attaches a managed policy to this principal.
* @param arn The ARN of the managed policy
* @param policy The managed policy
*/
attachManagedPolicy(arn: string): void;
addManagedPolicy(policy: IManagedPolicy): void;
}
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-iam/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export * from './lazy-role';
export * from './principals';
export * from './identity-base';
export * from './grant';
export * from './imported-resource-principal';
export * from './unknown-principal';

// AWS::IAM CloudFormation Resources:
export * from './iam.generated';
13 changes: 7 additions & 6 deletions packages/@aws-cdk/aws-iam/lib/lazy-role.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import cdk = require('@aws-cdk/cdk');
import { Grant } from './grant';
import { IManagedPolicy } from './managed-policy';
import { Policy } from './policy';
import { PolicyStatement } from './policy-document';
import { IPrincipal, PrincipalPolicyFragment } from './principals';
Expand Down Expand Up @@ -27,7 +28,7 @@ export class LazyRole extends cdk.Resource implements IRole {
private role?: Role;
private readonly statements = new Array<PolicyStatement>();
private readonly policies = new Array<Policy>();
private readonly managedPolicies = new Array<string>();
private readonly managedPolicies = new Array<IManagedPolicy>();

constructor(scope: cdk.Construct, id: string, private readonly props: LazyRoleProps) {
super(scope, id);
Expand Down Expand Up @@ -61,13 +62,13 @@ export class LazyRole extends cdk.Resource implements IRole {

/**
* Attaches a managed policy to this role.
* @param arn The ARN of the managed policy to attach.
* @param policy The managed policy to attach.
*/
public attachManagedPolicy(arn: string): void {
public addManagedPolicy(policy: IManagedPolicy): void {
if (this.role) {
this.role.attachManagedPolicy(arn);
this.role.addManagedPolicy(policy);
} else {
this.managedPolicies.push(arn);
this.managedPolicies.push(policy);
}
}

Expand Down Expand Up @@ -110,7 +111,7 @@ export class LazyRole extends cdk.Resource implements IRole {
const role = new Role(this, 'Default', this.props);
this.statements.forEach(role.addToPolicy.bind(role));
this.policies.forEach(role.attachInlinePolicy.bind(role));
this.managedPolicies.forEach(role.attachManagedPolicy.bind(role));
this.managedPolicies.forEach(role.addManagedPolicy.bind(role));
this.role = role;
}
return this.role;
Expand Down
62 changes: 40 additions & 22 deletions packages/@aws-cdk/aws-iam/lib/managed-policy.ts
Original file line number Diff line number Diff line change
@@ -1,30 +1,48 @@
import cdk = require('@aws-cdk/cdk');
import { Stack } from '@aws-cdk/cdk';
import { IResolveContext, Lazy, Stack } from '@aws-cdk/cdk';

/**
* A policy managed by AWS
*
* For this managed policy, you only need to know the name to be able to use it.
*
* Some managed policy names start with "service-role/", some start with
* "job-function/", and some don't start with anything. Do include the
* prefix when constructing this object.
* A managed policy
*/
export class AwsManagedPolicy {
constructor(private readonly managedPolicyName: string, private readonly scope: cdk.IConstruct) {
}
export interface IManagedPolicy {
/**
* The ARN of the managed policy
*/
readonly managedPolicyArn: string;
}

/**
* Managed policy
*
* This class is an incomplete placeholder class, and exists only to get access
* to AWS Managed policies.
*/
export class ManagedPolicy {
/**
* The Arn of this managed policy
* Construct a managed policy from one of the policies that AWS manages
*
* For this managed policy, you only need to know the name to be able to use it.
*
* Some managed policy names start with "service-role/", some start with
* "job-function/", and some don't start with anything. Do include the
* prefix when constructing this object.
*/
public get policyArn(): string {
// the arn is in the form of - arn:aws:iam::aws:policy/<policyName>
return Stack.of(this.scope).formatArn({
service: "iam",
region: "", // no region for managed policy
account: "aws", // the account for a managed policy is 'aws'
resource: "policy",
resourceName: this.managedPolicyName
});
public static fromAwsManagedPolicyName(managedPolicyName: string): IManagedPolicy {
class AwsManagedPolicy implements IManagedPolicy {
public readonly managedPolicyArn = Lazy.stringValue({
produce(ctx: IResolveContext) {
return Stack.of(ctx.scope).formatArn({
service: "iam",
region: "", // no region for managed policy
account: "aws", // the account for a managed policy is 'aws'
resource: "policy",
resourceName: managedPolicyName
});
}
});
}
return new AwsManagedPolicy();
}

protected constructor() {
}
}
21 changes: 11 additions & 10 deletions packages/@aws-cdk/aws-iam/lib/role.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { Construct, PhysicalName, Resource, ResourceIdentifiers, Stack } from '@aws-cdk/cdk';
import { Construct, Lazy, PhysicalName, Resource, ResourceIdentifiers, Stack } from '@aws-cdk/cdk';
import { Grant } from './grant';
import { CfnRole } from './iam.generated';
import { IIdentity } from './identity-base';
import { IManagedPolicy } from './managed-policy';
import { Policy } from './policy';
import { PolicyDocument, PolicyStatement } from './policy-document';
import { ArnPrincipal, IPrincipal, PrincipalPolicyFragment } from './principals';
import { AttachedPolicies, undefinedIfEmpty } from './util';
import { AttachedPolicies } from './util';

export interface RoleProps {
/**
Expand Down Expand Up @@ -33,7 +34,7 @@ export interface RoleProps {
*
* @default - No managed policies.
*/
readonly managedPolicyArns?: string[];
readonly managedPolicies?: IManagedPolicy[];

/**
* A list of named policies to inline into this role. These policies will be
Expand Down Expand Up @@ -125,7 +126,7 @@ export class Role extends Resource implements IRole {
// FIXME: Add warning that we're ignoring this
}

public attachManagedPolicy(_arn: string): void {
public addManagedPolicy(_policy: IManagedPolicy): void {
// FIXME: Add warning that we're ignoring this
}

Expand Down Expand Up @@ -186,7 +187,7 @@ export class Role extends Resource implements IRole {
public readonly policyFragment: PrincipalPolicyFragment;

private defaultPolicy?: Policy;
private readonly managedPolicyArns: string[];
private readonly managedPolicies: IManagedPolicy[] = [];
private readonly attachedPolicies = new AttachedPolicies();

constructor(scope: Construct, id: string, props: RoleProps) {
Expand All @@ -195,13 +196,13 @@ export class Role extends Resource implements IRole {
});

this.assumeRolePolicy = createAssumeRolePolicy(props.assumedBy, props.externalId);
this.managedPolicyArns = props.managedPolicyArns || [ ];
this.managedPolicies.push(...props.managedPolicies || []);

validateMaxSessionDuration(props.maxSessionDurationSec);

const role = new CfnRole(this, 'Resource', {
assumeRolePolicyDocument: this.assumeRolePolicy as any,
managedPolicyArns: undefinedIfEmpty(() => this.managedPolicyArns),
managedPolicyArns: Lazy.listValue({ produce: () => this.managedPolicies.map(p => p.managedPolicyArn) }, { omitEmpty: true }),
policies: _flatten(props.inlinePolicies),
path: props.path,
roleName: this.physicalName.value,
Expand Down Expand Up @@ -252,10 +253,10 @@ export class Role extends Resource implements IRole {

/**
* Attaches a managed policy to this role.
* @param arn The ARN of the managed policy to attach.
* @param policy The the managed policy to attach.
*/
public attachManagedPolicy(arn: string) {
this.managedPolicyArns.push(arn);
public addManagedPolicy(policy: IManagedPolicy) {
this.managedPolicies.push(policy);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,17 @@ import { PolicyStatement } from './policy-document';
import { IPrincipal, PrincipalPolicyFragment } from './principals';

/**
* Properties for an ImportedResourcePrincipal
* Properties for an UnknownPrincipal
*/
export interface ImportedResourcePrincipalProps {
export interface UnknownPrincipalProps {
/**
* The resource the role proxy is for
*/
readonly resource: IConstruct;
}

/**
* A principal associated with an imported resource
* A principal for use in resources that need to have a role but it's unknown
*
* Some resources have roles associated with them which they assume, such as
* Lambda Functions, CodeBuild projects, StepFunctions machines, etc.
Expand All @@ -23,12 +23,12 @@ export interface ImportedResourcePrincipalProps {
* instead, which will add user warnings when statements are attempted to be
* added to it.
*/
export class ImportedResourcePrincipal implements IPrincipal {
export class UnknownPrincipal implements IPrincipal {
public readonly assumeRoleAction: string = 'sts:AssumeRole';
public readonly grantPrincipal: IPrincipal;
private readonly resource: IConstruct;

constructor(props: ImportedResourcePrincipalProps) {
constructor(props: UnknownPrincipalProps) {
this.resource = props.resource;
this.grantPrincipal = this;
}
Expand Down
Loading