Skip to content

Commit

Permalink
fix(pipelines): cannot use constructs in build environment (#10654)
Browse files Browse the repository at this point in the history
The environment serialization that is used to calculate a hash
of the project configuration which is used to restart the pipeline
(still following?) was not taking into account that some of the
fields of `Environment` are not just data, but are objects.

We have to turn those into data before we can serialize them.

Fixes #10535.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
rix0rrr authored Oct 6, 2020
1 parent d345919 commit bf2c629
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 5 deletions.
17 changes: 16 additions & 1 deletion packages/@aws-cdk/pipelines/lib/synths/simple-synth-action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ export class SimpleSynthAction implements codepipeline.IAction, iam.IGrantable {
// here because the pipeline will definitely restart if projectName changes.
// (Resolve tokens)
const projectConfigHash = hash(Stack.of(scope).resolve({
environment,
environment: serializeBuildEnvironment(environment),
buildSpecString: buildSpec.toBuildSpec(),
environmentVariables,
}));
Expand Down Expand Up @@ -486,3 +486,18 @@ function hash<A>(obj: A) {
d.update(JSON.stringify(obj));
return d.digest('hex');
}

/**
* Serialize a build environment to data (get rid of constructs & objects), so we can JSON.stringify it
*/
function serializeBuildEnvironment(env: codebuild.BuildEnvironment) {
return {
privileged: env.privileged,
environmentVariables: env.environmentVariables,
type: env.buildImage?.type,
imageId: env.buildImage?.imageId,
computeType: env.computeType,
imagePullPrincipalType: env.buildImage?.imagePullPrincipalType,
secretsManagerArn: env.buildImage?.secretsManagerCredentials?.secretArn,
};
}
1 change: 1 addition & 0 deletions packages/@aws-cdk/pipelines/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
"nodeunit": "^0.11.3",
"pkglint": "0.0.0",
"@aws-cdk/aws-s3": "0.0.0",
"@aws-cdk/aws-ecr": "0.0.0",
"@aws-cdk/aws-ecr-assets": "0.0.0"
},
"peerDependencies": {
Expand Down
20 changes: 20 additions & 0 deletions packages/@aws-cdk/pipelines/test/builds.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import '@aws-cdk/assert/jest';
import * as cbuild from '@aws-cdk/aws-codebuild';
import * as codepipeline from '@aws-cdk/aws-codepipeline';
import * as ec2 from '@aws-cdk/aws-ec2';
import * as ecr from '@aws-cdk/aws-ecr';
import * as s3 from '@aws-cdk/aws-s3';
import { Stack } from '@aws-cdk/core';
import * as cdkp from '../lib';
Expand Down Expand Up @@ -405,6 +406,25 @@ test('SimpleSynthAction is IGrantable', () => {
});
});

test('SimpleSynthAction can reference an imported ECR repo', () => {
// Repro from https://github.com/aws/aws-cdk/issues/10535

// WHEN
new TestGitHubNpmPipeline(pipelineStack, 'Cdk', {
sourceArtifact,
cloudAssemblyArtifact,
synthAction: cdkp.SimpleSynthAction.standardNpmSynth({
sourceArtifact,
cloudAssemblyArtifact,
environment: {
buildImage: cbuild.LinuxBuildImage.fromEcrRepository(
ecr.Repository.fromRepositoryName(pipelineStack, 'ECRImage', 'my-repo-name'),
),
},
}),
});
});

function npmYarnBuild(npmYarn: string) {
if (npmYarn === 'npm') { return cdkp.SimpleSynthAction.standardNpmSynth; }
if (npmYarn === 'yarn') { return cdkp.SimpleSynthAction.standardYarnSynth; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,7 @@
"ProjectName": {
"Ref": "PipelineBuildSynthCdkBuildProject6BEFA8E6"
},
"EnvironmentVariables": "[{\"name\":\"_PROJECT_CONFIG_HASH\",\"type\":\"PLAINTEXT\",\"value\":\"7f09efece2ae66563f27569c480f9602225200051089f57486c5fa0b52095956\"}]"
"EnvironmentVariables": "[{\"name\":\"_PROJECT_CONFIG_HASH\",\"type\":\"PLAINTEXT\",\"value\":\"ea30b07764cb215963f4b0c292b5a06993fc3dd94621fbdc1c618e0f1e0dae10\"}]"
},
"InputArtifacts": [
{
Expand Down Expand Up @@ -1695,4 +1695,4 @@
}
}
}
}
}
4 changes: 2 additions & 2 deletions packages/@aws-cdk/pipelines/test/integ.pipeline.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@
"ProjectName": {
"Ref": "PipelineBuildSynthCdkBuildProject6BEFA8E6"
},
"EnvironmentVariables": "[{\"name\":\"_PROJECT_CONFIG_HASH\",\"type\":\"PLAINTEXT\",\"value\":\"7f09efece2ae66563f27569c480f9602225200051089f57486c5fa0b52095956\"}]"
"EnvironmentVariables": "[{\"name\":\"_PROJECT_CONFIG_HASH\",\"type\":\"PLAINTEXT\",\"value\":\"ea30b07764cb215963f4b0c292b5a06993fc3dd94621fbdc1c618e0f1e0dae10\"}]"
},
"InputArtifacts": [
{
Expand Down Expand Up @@ -1391,4 +1391,4 @@
}
}
}
}
}

0 comments on commit bf2c629

Please sign in to comment.