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

feat(ec2): add options for Cfn init proxies #19480

Closed
wants to merge 15 commits into from
Closed

feat(ec2): add options for Cfn init proxies #19480

wants to merge 15 commits into from

Conversation

maafk
Copy link
Contributor

@maafk maafk commented Mar 20, 2022

Closes #19479


All Submissions:

Adding new Unconventional Dependencies:

  • This PR adds new unconventional dependencies following the process described here

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@gitpod-io
Copy link

gitpod-io bot commented Mar 20, 2022

@github-actions github-actions bot added the @aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud label Mar 20, 2022
@maafk
Copy link
Contributor Author

maafk commented Mar 22, 2022

@corymhall I must not be understanding the prlint failure I'm getting

Error: Features must contain a change to an integration test file

I've tried adding and altering integration tests and continue to get this error. Am I missing something simple?

Thanks!

@peterwoodworth
Copy link
Contributor

@maafk I've put in a fix for this #19521, hang tight!

@maafk
Copy link
Contributor Author

maafk commented Mar 23, 2022

Integration test was updated, but erroring due to different orders in an IAM policy.

@aws-cdk/aws-ec2: Verifying integ.instance-init.js against integ.instance-init.expected.json ... CHANGED.
@aws-cdk/aws-ec2: Resources
@aws-cdk/aws-ec2: [~] AWS::IAM::Policy Instance2InstanceRoleDefaultPolicy610B37CD 
@aws-cdk/aws-ec2:  └─ [~] PolicyDocument
@aws-cdk/aws-ec2:      └─ [~] .Statement:
@aws-cdk/aws-ec2:          └─ @@ -1,8 +1,8 @@
@aws-cdk/aws-ec2:             [ ] [
@aws-cdk/aws-ec2:             [ ]   {
@aws-cdk/aws-ec2:             [ ]     "Action": [
@aws-cdk/aws-ec2:             [+]       "s3:GetBucket*",
@aws-cdk/aws-ec2:             [ ]       "s3:GetObject*",
@aws-cdk/aws-ec2:             [-]       "s3:GetBucket*",
@aws-cdk/aws-ec2:             [ ]       "s3:List*"
@aws-cdk/aws-ec2:             [ ]     ],
@aws-cdk/aws-ec2:             [ ]     "Effect": "Allow",
@aws-cdk/aws-ec2:             @@ -18,7 +18,8 @@
@aws-cdk/aws-ec2:             [ ]       ":s3:::",
@aws-cdk/aws-ec2:             [ ]       {
@aws-cdk/aws-ec2:             [ ]         "Ref": "Asset1S3Bucket"
@aws-cdk/aws-ec2:             [-]       }
@aws-cdk/aws-ec2:             [+]       },
@aws-cdk/aws-ec2:             [+]       "/*"
@aws-cdk/aws-ec2:             [ ]     ]
@aws-cdk/aws-ec2:             [ ]   ]
@aws-cdk/aws-ec2:             [ ] },
@aws-cdk/aws-ec2:             @@ -33,8 +34,7 @@
@aws-cdk/aws-ec2:             [ ]       ":s3:::",
@aws-cdk/aws-ec2:             [ ]       {
@aws-cdk/aws-ec2:             [ ]         "Ref": "Asset1S3Bucket"
@aws-cdk/aws-ec2:             [-]       },
@aws-cdk/aws-ec2:             [-]       "/*"
@aws-cdk/aws-ec2:             [+]       }
@aws-cdk/aws-ec2:             [ ]     ]
@aws-cdk/aws-ec2:             [ ]   ]
@aws-cdk/aws-ec2:             [ ] }

These are exactly the same policies, but synthesized with actions/resources in a different order

@maafk
Copy link
Contributor Author

maafk commented Apr 6, 2022

@corymhall Anything I can clarify on this PR?

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
  • Commit ID: 32bf602
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

*
* @default
*/
readonly httpsProxy?: string;
Copy link
Contributor

@LukvonStrom LukvonStrom Apr 23, 2022

Choose a reason for hiding this comment

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

as these are secrets, I would like to propose an alternative way:
An interface like this:

enum ProxyProtocol {
HTTP= "http",
HTTPS = "https",
}

interface IProxyConfig {
protocol: ProxyProtocol,
user?: string,
password?: ISecret,
host: string,
port?: number
}


// [...]

readonly proxy?: IProxyConfig;

take only one argument called proxy here and decide wether -http-proxy or -https-proxy is appropriate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Finally getting back to this. Currently figuring out how to deal with circular dependencies between packages.

aws-secretsmanager has a dependency on the aws-ec2 package, causing issues when doing a yarn build within the aws-ec2 package after adding secrets manager.

Trying to figure out how to handle this if possible

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Articulated this in #19479

@github-actions
Copy link

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

5 similar comments
@github-actions
Copy link

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

@github-actions
Copy link

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

@github-actions
Copy link

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

@github-actions
Copy link

github-actions bot commented May 1, 2022

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

@github-actions
Copy link

github-actions bot commented May 2, 2022

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

@github-actions
Copy link

github-actions bot commented May 3, 2022

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

@github-actions
Copy link

github-actions bot commented May 4, 2022

This PR has been in the BUILD FAILING state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

@github-actions
Copy link

This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error.

@github-actions github-actions bot added the closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. label May 11, 2022
@github-actions github-actions bot closed this May 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(ec2): allow --http-proxy and --https-proxy in Cloudformation helper scripts
5 participants