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

chore(ec2): update WindowsVersions enum #29796

Open
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

nmussy
Copy link
Contributor

@nmussy nmussy commented Apr 11, 2024

Issue # (if applicable)

None

Reason for this change

Update the CDK EC2 listed Windows versions to match the current availability. Follow up to #29435, which was reverted for erroneous changes.
This version in based on the /aws/service/ami-windows-latest/* SSM parameters, similarly to #29738.

Description of changes

  • Updates to the WindowsVersion enum:
    • 38b6cd9: Added new versions
    • f21fd86: Deprecated EOL versions
    • 686d8d3: Added unit test to verify that all versions keys and values match each other. This highlighted both several typos in keys and values, and allowed for a simpler reflection process in the integration test
  • Added jsii and jsii-reflect dependencies to @aws-cdk-testing/framework-integ
    • This allows the integration test to check whether a field is deprecated

Description of how you validated changes

The WindowsVersion values were compared to the SDK results of ssm:GetParametersByPath with the following params:

{
	"Path": "/aws/service/ami-windows-latest"
}

The parameters that did not start with /aws/service/ami-windows-latest/Windows_Server were ignored. Some are Amazon Linux images:

  • amzn2-ami-hvm-2.0.*
  • amzn2-x86_64-SQL_2019_*

Others are either EC2LaunchV2 or NitroTPM Windows images, neither currently supported by the CDK:

  • EC2LaunchV2-Windows_Server-2016-English-*
  • TPM-Windows_Server-2016-English-*
  • TPM-Windows_Server-2019-English-*
  • TPM-Windows_Server-2022-English-*

Also went a little crazy on the integration, it checks all of the listed WindowsVersion and:

  • Expects all non-deprecated versions to have an SSM /aws/service/ami-windows-latest/<version> parameter
  • Expects all deprecated versions not to have that same SSM parameter

This ensures that a situation like #29736 (comment) does not happen again

Checklist


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

@github-actions github-actions bot added the p2 label Apr 11, 2024
@aws-cdk-automation aws-cdk-automation requested a review from a team April 11, 2024 19:05
@github-actions github-actions bot added the distinguished-contributor [Pilot] contributed 50+ PRs to the CDK label Apr 11, 2024
@aws-cdk-automation aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Apr 11, 2024
@aaythapa aaythapa assigned aaythapa and unassigned aaythapa Apr 18, 2024
@aws-cdk-automation
Copy link
Collaborator

This PR has been in the MERGE CONFLICTS 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.

@aws-cdk-automation
Copy link
Collaborator

This PR has been in the MERGE CONFLICTS 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.

@aws-cdk-automation
Copy link
Collaborator

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.

@aws-cdk-automation
Copy link
Collaborator

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.

@pahud
Copy link
Contributor

pahud commented Jul 30, 2024

Hi

This PR has been pending for community review for a while. Please consider posting it on the #contributing channel in cdk.dev. Community members will be on the lookout there as well for possible reviews.

Check How to get your P2 PR community reviewed for more details.

@aws-cdk-automation
Copy link
Collaborator

This PR has been in the MERGE CONFLICTS 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.

@aws-cdk-automation
Copy link
Collaborator

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.

@aws-cdk-automation aws-cdk-automation added the closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. label Sep 1, 2024
Copy link

github-actions bot commented Sep 1, 2024

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 1, 2024
@aws-cdk-automation aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Sep 1, 2024
@aaythapa aaythapa reopened this Dec 9, 2024
@aaythapa aaythapa requested a review from a team as a code owner December 9, 2024 17:36
@aws aws unlocked this conversation Dec 9, 2024
Copy link

codecov bot commented Dec 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.66%. Comparing base (62a0638) to head (1e8b376).

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #29796   +/-   ##
=======================================
  Coverage   78.66%   78.66%           
=======================================
  Files         107      107           
  Lines        7161     7161           
  Branches     1320     1320           
=======================================
  Hits         5633     5633           
  Misses       1343     1343           
  Partials      185      185           
Flag Coverage Δ
suite.unit 78.66% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
packages/aws-cdk 78.66% <ø> (ø)

@aws-cdk-automation aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Dec 9, 2024
@aws-cdk-automation
Copy link
Collaborator

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.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: 1e8b376
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. distinguished-contributor [Pilot] contributed 50+ PRs to the CDK p2 pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants