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

[pipelines] Self mutation deploys everything not just the pipeline #9669

Closed
jonny-rimek opened this issue Aug 13, 2020 · 7 comments
Closed
Assignees
Labels
@aws-cdk/pipelines CDK Pipelines library bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.

Comments

@jonny-rimek
Copy link

jonny-rimek commented Aug 13, 2020

The update pipeline deploys changes that are made outside of the pipeline, e.g. creating a s3 bucket

this is the pipeline that got created
Screenshot_2020-08-13 CodePipeline - AWS Developer Tools

to my surprise it doesn't contain deploy stages, which is what I expected from the documentation and examples

#!/usr/bin/env node
import 'source-map-support/register';
import cdk = require('@aws-cdk/core');
import { V2 } from '../lib/v2';
import { Construct, Stage, Stack, StackProps, StageProps, SecretValue } from '@aws-cdk/core';
import { CdkPipeline, SimpleSynthAction } from '@aws-cdk/pipelines';
import * as codepipeline from '@aws-cdk/aws-codepipeline';
import * as codepipeline_actions from '@aws-cdk/aws-codepipeline-actions'

class Wowmate extends Stage {
	constructor(scope: Construct, id: string, props?: StageProps) {
		super(scope, id, props);

		new V2(this, 'V2')
		// new Frontend(this, 'frontend')
	}
}

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

    const sourceArtifact = new codepipeline.Artifact();
    const cloudAssemblyArtifact = new codepipeline.Artifact();

    const pipeline = new CdkPipeline(this, 'Pipeline', {
      pipelineName: 'WowmatePipeline',
      cloudAssemblyArtifact,

      sourceAction: new codepipeline_actions.GitHubSourceAction({
		actionName: 'GitHub',
		output: sourceArtifact,
		branch: 'master',
		oauthToken: SecretValue.secretsManager('github-personal-access-token'),
		//TODO: switch to webhook, might need to update the oath token
        trigger: codepipeline_actions.GitHubTrigger.POLL,
        owner: 'jonny-rimek',
        repo: 'wowmate',
      }),
      synthAction: SimpleSynthAction.standardNpmSynth({
        sourceArtifact,
        cloudAssemblyArtifact,

        buildCommand: 'npm run build',
      }),
    });

	const wmp = pipeline.addApplicationStage(new Wowmate(this, 'Prod'));
	wmp.addManualApprovalAction();
  }
}


const app = new cdk.App();
new WowmatePipelineStack(app, 'WoWM', {
	env: {region: "us-east-1"}
});

Screenshot_2020-08-13 CodeBuild - AWS Developer Tools(1)

Currently the v2 stack only contains 2 buckets, if I add an ApplicationLoadBalancedFargateService, it tried to build docker in this stage too, instead of creating a dedicated stage, which is the desired behaviour, afaik.

Environment

  • **Framework Version:1.58

This is 🐛 Bug Report

as far as I can tell I followed all the tutorials to the letter, I have no idea whats wrong

@jonny-rimek jonny-rimek added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Aug 13, 2020
@github-actions github-actions bot added the @aws-cdk/pipelines CDK Pipelines library label Aug 13, 2020
@rix0rrr
Copy link
Contributor

rix0rrr commented Aug 17, 2020

As far as I can tell this ticket contains 3 bug reports:

The update pipeline deploys changes that are made outside of the pipeline, e.g. creating a s3 bucket

I don't know what that means. Can you explain what you mean by "made outside of the pipeline"?

to my surprise it doesn't contain deploy stages

That is odd. I'm suprised by the fact that you didn't add an account, even if only to look like account: process.env.CDK_DEFAULT_ACCOUNT, and I'm even more suprised by the fact that you didn't get yelled at for not supplying an account. Can you verify whether that makes a difference?

Other than that, is there a place I can find this code to play with it for myself?

Currently the v2 stack only contains 2 buckets, if I add an ApplicationLoadBalancedFargateService, it tried to build docker in this stage too, instead of creating a dedicated stage, which is the desired behaviour, afaik.

It will not be a dedicated stage; Stages contain Stacks contain Constructs. If you put the ApplicationLoadBalancedFargateService into the same stack as your S3 buckets, that's where it will go. I don't understand what "it tried to build docker" means.

@jonny-rimek
Copy link
Author

jonny-rimek commented Aug 17, 2020

I don't know what that means. Can you explain what you mean by "made outside of the pipeline"?

My understanding is that the UpdatePipeline Part of the Pipeline is where the pipeline rebuilds itself. One problem is that it not only adjusts the pipeline, but deploys all the infrastructure inside the UpdatePipeline part, i.e. the Bucket and tries to build the docker image and deploy it, but that fails, which is probably not really a problem because it shouldn't run at this point anyway.

That is odd. I'm surprised by the fact that you didn't add an account, even if only to look like account: process.env.CDK_DEFAULT_ACCOUNT, and I'm even more surprised by the fact that you didn't get yelled at for not supplying an account. Can you verify whether that makes a difference?

I'm adding my account ID and will check if that changes anything.

It will not be a dedicated stage; Stages contain Stacks contain Constructs. If you put the ApplicationLoadBalancedFargateService into the same stack as your S3 buckets, that's where it will go. I don't understand what "it tried to build docker" means.

with stage, I meant a dedicated step inside the Code Pipeline, not a stage in CDK ^^

Other than that, is there a place I can find this code to play with it for myself?

@rix0rrr I'll extract the code and post the repo soon

If the pipeline construct will go stable, having a somewhat complex example would go a long way to understanding how it all works

@jonny-rimek
Copy link
Author

As far as I can tell, adding the account ID didn't change anything. I still don't have the deploy step in CodePipeline and the UpdatePipeline step still applies infrastructure changes besides the pipeline itself (e.g. a new bucket inside the v2 construct).

repo with the code:
https://github.com/jonny-rimek/jonny

@jonny-rimek
Copy link
Author

Not sure if I missed some news, but is it possible to publish assets(ecs/lambda) during synth and deploy it with Code Deploy, without the pipelines construct. I used Circle CI in the past, but running cdk deploy as a last step in the pipeline has a couple of problems.

@rix0rrr
Copy link
Contributor

rix0rrr commented Aug 18, 2020

@rix0rrr
Copy link
Contributor

rix0rrr commented Aug 18, 2020

Not sure if I missed some news, but is it possible to publish assets(ecs/lambda) during synth and deploy it with Code Deploy, without the pipelines construct. I used Circle CI in the past, but running cdk deploy as a last step in the pipeline has a couple of problems.

It might be possible, but has some assembly required. After cdk synth, you run cdk-assets to upload the assets. Not sure there's a flag to do the template deploy without assets, so you might need to use the AWS CLI to do the actual deploy.

@rix0rrr rix0rrr closed this as completed Aug 18, 2020
@jonny-rimek
Copy link
Author

@rix0rrr such a silly mistake^^ thanks for the help. It definitely creates the prepare and the deploy step.

I'm stopping using this construct for now until it is stable.

rix0rrr added a commit that referenced this issue Sep 3, 2020
In current recommended CDK Pipelines usage, the construct tree looks
like this:

```
App -> Stack1 -> Stage -> Stack2 -> Resource
```

It's an easy mistake to forget `Stack2` (or make it a generic
`Construct` instead), in which case the hierachy looks like the one
below and `Stack1` will render Resource in its CloudFormation template:

```
App -> Stack1 -> Stage -> Construct -> Resource
```

This case should not have been allowed in the first place: `Stage`s
define a new assembly scope and so `Stack1` does not exist from the
point of view of `Resource`.

Change `Stack.of()` to fail to find `Stack1` in this case, so
that it becomes illegal to define `Resource` in this position.

Fixes #9792, relates to #9669.
mergify bot pushed a commit that referenced this issue Sep 8, 2020
…0156)

In current recommended CDK Pipelines usage, the construct tree looks
like this:

```
App -> Stack1 -> Stage -> Stack2 -> Resource
```

It's an easy mistake to forget `Stack2` (or make it a generic
`Construct` instead), in which case the hierachy looks like the one
below and `Stack1` will render Resource in its CloudFormation template:

```
App -> Stack1 -> Stage -> Construct -> Resource
```

This case should not have been allowed in the first place: `Stage`s
define a new assembly scope and so `Stack1` does not exist from the
point of view of `Resource`.

Change `Stack.of()` to fail to find `Stack1` in this case, so
that it becomes illegal to define `Resource` in this position.

Fixes #9792, relates to #9669.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/pipelines CDK Pipelines library bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Development

No branches or pull requests

2 participants