Skip to content

Conversation

@aemous
Copy link
Contributor

@aemous aemous commented Dec 23, 2025

Description of changes:

  • Add general architecture for creating new rules that do not suggest automatic fixes. Instead, these rules flag commands and suggest manual actions to take.
  • Add new ECR GetLogin rule, which detects ecr get-login commands. If found, flags them as needing manual review.
  • Update existing linting rules for better detection, including replacing brittle string patterns with more complex rules on the abstract syntax tree.
    • As a concrete example of a command that was previously missed but is now captured, aws --debug s3 cp s3://$SRC_BUCKET s3://$DEST_BUCKET. This would have been missed previously due to the --debug flag before the service subcommand, and the use of concatenation with the word s3:// and variable $SRC_BUCKET.
  • Modify display_finding to make better use of difflib. Removes unneeded code.
  • Add new tests covering the changes described above.

Description of tests:

  • Ran and passed all tests.
  • Successfully ran manual tests using the linter against test scripts.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@aemous aemous requested a review from a team December 23, 2025 21:45
@codecov-commenter
Copy link

codecov-commenter commented Dec 23, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.33%. Comparing base (c2db7d0) to head (33e490a).
⚠️ Report is 2 commits behind head on upgrade-linting-tool.

Additional details and impacted files
@@                  Coverage Diff                  @@
##           upgrade-linting-tool    #9948   +/-   ##
=====================================================
  Coverage                 93.33%   93.33%           
=====================================================
  Files                       209      209           
  Lines                     16807    16807           
=====================================================
  Hits                      15687    15687           
  Misses                     1120     1120           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@aemous
Copy link
Contributor Author

aemous commented Dec 23, 2025

Question for reviewer @hssyoo

Current state: We planned to merge the linter to the develop branch. This implies customers of v1 using our GH repo will now see the linter files in their repositories.

What are our thoughts on instead merge the linter to its own separate v1v2-linter branch? This branch could potentially be an orphan branch, which means it has a clean history, and does not branch off of any other branch. Alternatively, it can branch from develop, and then we can delete all CLI v1 related files from the linter branch. This also enabled us to cleanly have GH Actions that run only in the linter branch.

@hssyoo
Copy link
Contributor

hssyoo commented Dec 24, 2025

Question for reviewer @hssyoo

Current state: We planned to merge the linter to the develop branch. This implies customers of v1 using our GH repo will now see the linter files in their repositories.

What are our thoughts on instead merge the linter to its own separate v1v2-linter branch? This branch could potentially be an orphan branch, which means it has a clean history, and does not branch off of any other branch. Alternatively, it can branch from develop, and then we can delete all CLI v1 related files from the linter branch. This also enabled us to cleanly have GH Actions that run only in the linter branch.

Separate branch sounds good to me. Feel free to run with the option you think makes sense.

Copy link
Contributor

@hssyoo hssyoo left a comment

Choose a reason for hiding this comment

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

Looks good, just need to refine the ECR rule. Another bug that was missed in the last PR is that the hidden alias rule appends the alternative the command rather than replacing the hidden alias. eg:

-aws lightsail import-key-pair --key-pair-name mykey --public-key-base-64 c3NoLXJzYQ==
+aws lightsail import-key-pair --key-pair-name mykey --public-key-base-64 c3NoLXJzYQ== --public-key-base64

@aemous aemous merged commit c7cbbe1 into aws:upgrade-linting-tool Dec 24, 2025
48 checks passed
aemous added a commit that referenced this pull request Jan 5, 2026
Includes all changes from #9803, #9859, #9936, #9948, and #9953.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants