-
Notifications
You must be signed in to change notification settings - Fork 7
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: add action for assisting with badge verification and migrate to nginx #191
Conversation
corang
commented
Jul 31, 2024
•
edited
Loading
edited
- move badge logic to uds task
- conditional logic based on github ci or not
- change example package to nginx
- all 3 flavors
- behind authservice or sso?
- extend badge logic to work for multi ns/package crd
- Summary at end of badge script
- More validations:
- package version matches app version in each flavor
- group sections for gh actions (headers?)
- better kubectl-validate pathing for binary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two overall suggestions
- change everything to "warning" right now. There are assumptions made that may not be accurate.
- shift logic to tasks.yaml - "builders" should be able to verify all of this easily prior to CI checkout.
Testing out with warnings on packages is a great start, but we should be wary of complication. Should some things shift to elsewhere in the stack? (like netpols from pepr for example or /healthz, ready endpoints)
I wanted to publicly add that I love this work @corang - it's exactly the direction and shape of work I was hoping to see in Marketplace and it happened with exactly zero nudging from me. ❤️ 🦄 |
|
@corang Isn't it easier to test as tasks you can run locally vs github actions? Copy on long-term desire. Keep in mind the in-portability aspect of doing this in github actions? What if a vendor is integrating and they dont' use github for example? |
Added a new issue #220 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm with those two tech debt issues