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): pipeline type v2 #29146

Closed
1 of 2 tasks
wcb-pont opened this issue Feb 17, 2024 · 8 comments
Closed
1 of 2 tasks

(pipelines): pipeline type v2 #29146

wcb-pont opened this issue Feb 17, 2024 · 8 comments
Labels
@aws-cdk/pipelines CDK Pipelines library effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.

Comments

@wcb-pont
Copy link

Describe the feature

the aws_codepipeline package was update to allow pipeline type v2 (super excited, love it, have been waiting a long time for this)

I have services that use both the pipelines and aws_codepipeline package depending on the use case. I have very easily done the upgrade on my more complex pipelines and shortly will be doing some work for environment variables.

so 2 things:

  1. services that have simpler pipelines using the pipelines module are now getting this warning
    [Warning at example] V1 pipeline type is implicitly selected when `pipelineType` is not set. If you want to use V2 type, set `PipelineType.V2`. [ack: @aws-cdk/aws-codepipeline:unspecifiedPipelineType]
  • granted this is just a warning, but is a slight pain when dealing with logs and running grep for warnings and errors
  1. Is there anything in the pipelines team pipeline (if you will) for implementation
  • I have a very full schedule right now, running all arch/devops/devex/backend for my company (have worked every weekend for about 4 months now)
    • if there is nothing in the backlog, i might be able to put in a story to add the implementation in 2-3 weeks and submit the pr to you
    • if something is in the backlog for support in the pipelines package, is there a timeframe?

Hope this doesn't seem pushy, I'm just really excited that the first part came through and even more then wanting to get rid of the warnings. I would like to move a bunch of things to env variables instead of some of the hardcoded arns and ssm work arounds I have going on.

P.S. Got started working on the cognito pre token generation v2 for scopes transition this week and am finishing it up this weekend. like 19 thumbs up on that, makes my multi tenant (users can have multiple tenants also and different scopes per tenant) 1000000x simpler

Use Case

described in feature*

Proposed Solution

described in feature*

if its backlogged to be done shortly, I'm super excited. if not I can toss it in to my work in a few weeks and create a pr

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.128.0

Environment details (OS name and version, etc.)

n/a

@wcb-pont wcb-pont added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Feb 17, 2024
@github-actions github-actions bot added the @aws-cdk/pipelines CDK Pipelines library label Feb 17, 2024
@gossandr
Copy link
Contributor

I am also impacted by this issue and can offer you some additional information @wcb-pont

The issue, briefly summarized, is this:

If your only concern is the warning message, this is suppressible.

As an example (see the example included in pipelines for reference):

    const pipeline = new pipelines.CodePipeline(this, 'Pipeline', {
      synth: new pipelines.ShellStep('Synth', {
        // Use a connection created using the AWS console to authenticate to GitHub
        // Other sources are available.
        input: pipelines.CodePipelineSource.connection('my-org/my-app', 'main', {
          connectionArn: 'arn:aws:codestar-connections:us-east-1:222222222222:connection/7d2469ff-514a-4e4f-9003-5ca4a43cdc41', // Created using the AWS console * });',
        }),
        commands: [
          'npm ci',
          'npm run build',
          'npx cdk synth',
        ],
      }),
    });
    cdk.Annotations.of(pipeline).acknowledgeWarning('@aws-cdk/aws-codepipeline:unspecifiedPipelineType', 'Ignored because we know we are using v1');

This will suppress the warning.

BUT - we need this issue to be looked at so that support for the flag/feature can be added to pipelines. It would be much better to just set the PipelineType in pipelines.

@gossandr
Copy link
Contributor

I found something else sort of interesting in this commit: 40ffe2b#diff-8e15d23929b5ff6c128d73523da6a4322f975856fea30df4f969a21edf5f4eb2

The packages/aws-cdk-lib/pipelines/test/codepipeline/codepipeline.test.ts file (part of pipelines and not aws-codepipeline) was modified so that the test would pass when the features were added to aws-codepipeline to support v2. The test in this file appears to do nothing now as the expect() was removed.

I don't know if this was the most ideal choice. I am not sure what the lift is to have full support for v2 in the pipelines library, but it seems like it would be a relatively easy change to pass PipelineType = v1 and add a note to the readme that currently only supports v1 pipelines. This would resolve the warning. Alternatively, add the annotation in the pipelines library to suppress the warning

@pahud
Copy link
Contributor

pahud commented Feb 20, 2024

The warning seems to be from here:

// TODO: Change the default value of `pipelineType` to V2 under a feature flag.
if (props.pipelineType === undefined) {
Annotations.of(this).addWarningV2('@aws-cdk/aws-codepipeline:unspecifiedPipelineType', 'V1 pipeline type is implicitly selected when `pipelineType` is not set. If you want to use V2 type, set `PipelineType.V2`.');
}

And according to this, I believe you can acknowledge it using acknowledgeWarning() and looks like you have added that in your code.

@pahud pahud added p2 response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Feb 20, 2024
@gossandr
Copy link
Contributor

@pahud yes, but the issue is that the official CDK pipelines construct should be the one to either suppress this warning or set PipelineType to V1 when it creates the pipeline object using aws-codepipeline library.

I am glad that I can suppress it in my code, but would much rather have the ability to set that directly when i use pipelines. Right now that isn't an option, because pipelines was created long ago before v2 pipelines were a thing.

The original creator of this issue wants v2 support added to pipelines. Me personally, i'd be happy if pipelines at least for the short term set PipelineType to v1 when creating the pipeline "under its hood"

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Feb 20, 2024
@pahud
Copy link
Contributor

pahud commented Feb 21, 2024

related to #29190

@pahud
Copy link
Contributor

pahud commented Feb 21, 2024

I guess this issue can be closed #29199

@pahud pahud added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Feb 21, 2024
@wcb-pont
Copy link
Author

@pahud I am looking at that pr and it looks good. closing for #29199

Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

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 effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.
Projects
None yet
Development

No branches or pull requests

3 participants