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.

Sign up for GitHub

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

Add ability to assume multiple roles #178

Merged
merged 1 commit into from
Aug 2, 2022
Merged

Add ability to assume multiple roles #178

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions lib/ci-stack.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import {
} from '@aws-cdk/core';
import { ListenerCertificate } from '@aws-cdk/aws-elasticloadbalancingv2';
import { Bucket } from '@aws-cdk/aws-s3';
import { disconnect } from 'cluster';
import { CIConfigStack } from './ci-config-stack';
import { JenkinsMainNode } from './compute/jenkins-main-node';
import { JenkinsMonitoring } from './monitoring/ci-alarms';
Expand All @@ -41,7 +40,7 @@ export interface CIStackProps extends StackProps {
/** Do you want to retain jenkins jobs and build history */
readonly dataRetention?: boolean;
/** IAM role ARN to be assumed by jenkins agent nodes eg: cross-account */
readonly agentAssumeRole?: string;
readonly agentAssumeRole?: string[];
/** File path containing global environment variables to be added to jenkins enviornment */
readonly envVarsFilePath?: string;
/** Add Mac agent to jenkins */
Expand Down Expand Up @@ -69,8 +68,6 @@ export class CIStack extends Stack {
},
},
});

const agentAssumeRoleContext = `${props?.agentAssumeRole ?? this.node.tryGetContext('agentAssumeRole')}`;
const macAgentParameter = `${props?.macAgent ?? this.node.tryGetContext('macAgent')}`;

const useSslParameter = `${props?.useSsl ?? this.node.tryGetContext('useSsl')}`;
Expand Down Expand Up @@ -152,7 +149,7 @@ export class CIStack extends Stack {
adminUsers: props?.adminUsers,
agentNodeSecurityGroup: securityGroups.agentNodeSG.securityGroupId,
subnetId: vpc.publicSubnets[0].subnetId,
}, this.agentNodes, agentAssumeRoleContext.toString(), macAgentParameter.toString());
}, this.agentNodes, macAgentParameter.toString(), props?.agentAssumeRole);

const externalLoadBalancer = new JenkinsExternalLoadBalancer(this, {
vpc,
Expand Down
7 changes: 3 additions & 4 deletions lib/compute/agent-node-config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export class AgentNodeConfig {

public readonly SSHEC2KeySecretId: string;

constructor(stack: Stack, assumeRole: string) {
constructor(stack: Stack, assumeRole?: string[]) {
this.STACKREGION = stack.region;
this.ACCOUNT = stack.account;

Expand All @@ -61,7 +61,6 @@ export class AgentNodeConfig {

const ecrManagedPolicy = new ManagedPolicy(stack, 'OpenSearch-CI-AgentNodePolicy', {
description: 'Jenkins agents Node Policy',
managedPolicyName: 'OpenSearch-CI-AgentNodePolicy',
statements: [
new PolicyStatement({
effect: Effect.ALLOW,
Expand Down Expand Up @@ -107,11 +106,11 @@ export class AgentNodeConfig {
);

/* eslint-disable eqeqeq */
if (assumeRole.toString() !== 'undefined') {
if (assumeRole) {
Copy link
Member

Choose a reason for hiding this comment

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

The assumeRole is not a boolean, have you tested this @gaiksaya ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, works as expected. Also the tests cover this scenario here in this file test/compute/agent-node-config.test.ts

Copy link
Member Author

Choose a reason for hiding this comment

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

Also if the resource string array checks for the validity of the resource we are adding, it cannot be blank or random values has to be a valid resource like arns or *

// policy to allow assume role AssumeRole
AgentNodeRole.addToPolicy(
new PolicyStatement({
resources: [assumeRole],
resources: assumeRole,
actions: ['sts:AssumeRole'],
}),
);
Expand Down
2 changes: 1 addition & 1 deletion lib/compute/jenkins-main-node.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ export class JenkinsMainNode {
foundJenkinsProcessCount: Metric
}

constructor(stack: Stack, props: JenkinsMainNodeProps, agentNode: AgentNodeProps[], assumeRole: string, macAgent: string) {
constructor(stack: Stack, props: JenkinsMainNodeProps, agentNode: AgentNodeProps[], macAgent: string, assumeRole?: string[]) {
this.ec2InstanceMetrics = {
cpuTime: new Metric({
metricName: 'procstat_cpu_usage',
Expand Down
30 changes: 27 additions & 3 deletions test/compute/agent-node-config.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,11 @@

import { Stack, App } from '@aws-cdk/core';
import {
expect as expectCDK, haveResource, haveResourceLike, countResources,
expect as expectCDK, haveResourceLike,
} from '@aws-cdk/assert';
import { readFileSync } from 'fs';
import { load } from 'js-yaml';
import { CIStack } from '../../lib/ci-stack';
import { JenkinsMainNode } from '../../lib/compute/jenkins-main-node';

test('Agents Resource is present', () => {
const app = new App({
Expand Down Expand Up @@ -49,7 +48,6 @@ test('Agents Resource is present', () => {
expectCDK(stack).to(haveResourceLike('AWS::IAM::ManagedPolicy', {
Description: 'Jenkins agents Node Policy',
Path: '/',
ManagedPolicyName: 'OpenSearch-CI-AgentNodePolicy',
Roles: [
{
Ref: 'OpenSearchCIAgentNodeRole4270FE0F',
Expand Down Expand Up @@ -124,6 +122,32 @@ test('Agents Resource is present', () => {
}));
});

test('Agents Node policy with assume role Resource is present', () => {
const app = new App({
context: { useSsl: 'true', runWithOidc: 'true' },
});
const stack = new CIStack(app, 'TestStack', {
agentAssumeRole: ['arn:aws:iam::12345:role/test-role', 'arn:aws:iam::901523:role/test-role2'],
});

expectCDK(stack).to(haveResourceLike('AWS::IAM::Policy', {
PolicyDocument: {
Statement: [
{
Action: 'sts:AssumeRole',
Effect: 'Allow',
Resource: [
'arn:aws:iam::12345:role/test-role',
'arn:aws:iam::901523:role/test-role2',
],
},
],
Version: '2012-10-17',
},

}));
});

describe('JenkinsMainNode Config with macAgent template', () => {
// WHEN
const testYaml = 'test/data/jenkins.yaml';
Expand Down