Refactor Workflow Linter

[joseph-flinn]

🎟️ Tracking

🚧 Type of change

• 🧹 Tech debt (refactoring, code cleanup, dependency upgrades, etc.)

📔 Objective

Make the Workflow Linter more easily maintainable through abstracting Rules and creating Workflow, Job, and Step level objects to test against

📋 Code changes

This is almost a full rewrite. All of the files are new

📸 Screenshots

Output of no changes needed:

Output of failed jobs:

⏰ Reminders before review

• Contributor guidelines followed
• All formatters and local linters executed and passed
• Written new unit and / or integration tests where applicable
• Used internationalization (i18n) for all UI strings
• CI builds passed
• Communicated to DevOps any deployment requirements
• Updated any necessary documentation or informed the documentation team

🦮 Reviewer guidelines

• 👍 (:+1:) or similar for great changes
• 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
• ❓ (:question:) for questions
• 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
• 🎨 (:art:) for suggestions / improvements
• ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
• 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
• ⛏ (:pick:) for minor or nitpick changes

[mimartin12]

While in review, I saw some unused imports for some files such as cli.py and settings.py. Figured I would just drop a comment instead of try to point them all out with GitHub suggestions 😄

[joseph-flinn]

I think this is finally ready for review 😅. I'd like to push out packaging for PyPi to a followup task.

[mimartin12]

Nothing crazy from me on the changes. They look good.
I am curious how the reliability of the various usage of loading data from keys, with the assumption that the data will just be there.

For example, this assumes that there will be a value for tag_name, correct me if I am wrong, but I think we personally have run into situations where there wasn't a tag available. This would raise a KeyError that isn't handled by the application itself.

tag_name = json.loads(response.data)["tag_name"]

This isn't necessarily something that must change today, but something in the future that would improve the reliability of the linting application.

[joseph-flinn]

Yikes...we need to figure out that rate limiting issue...

[joseph-flinn]

I am curious how the reliability of the various usage of loading data from keys, with the assumption that the data will just be there.

@mimartin12 I definitely think that that resiliency should be built-in. This is a great callout. We shouldn't be assuming that there is data. I'll go back through and update these in all of the places that I can find.

[joseph-flinn]

After a quick scan, it seems like the major offender (and I believe the only offender) is src.bitwarden_workflow_linter.actions:get_latest_version which is interacting with the GitHub API

I think we are implicitly safe since all GitHub Release will have a tag and an associated tag_name. Similarly, there is a high chance that we are implicitly safe with the response from asking for the data associated with the tag. But to be on the safe side for now, I've wrapped the logic in a specific GitHubResponseSchemaError to alert us to a change in the GitHub Repsonse that would break the current logic. It doesn't fix the resiliency issue, but brings a bit more visibility if this happens