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

KEP validation seems to be brittle? #2470

Closed
BenTheElder opened this issue Feb 8, 2021 · 3 comments · Fixed by #2409
Closed

KEP validation seems to be brittle? #2470

BenTheElder opened this issue Feb 8, 2021 · 3 comments · Fixed by #2409
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture.

Comments

@BenTheElder
Copy link
Member

BenTheElder commented Feb 8, 2021

Verifying verify-kep-metadata.sh
--- FAIL: TestValidation (0.09s)
    --- FAIL: TestValidation/../../keps/sig-testing/2464-kubetest2-ci-migration/kep.yaml (0.00s)
        main_test.go:90: ../../keps/sig-testing/2464-kubetest2-ci-migration/kep.yaml has an error: error validating KEP metadata: "prr-approvers" must be one of (deads2k,johnbelamaric,wojtek-t) but it is a string: TBD

Fails on: #2465
But not on: #2420 related PRs.

IMHO this should not fail for provisional KEPs either, it seems reasonable to introduce a KEP provisionally before seeking PRR (not to mention #2311 / PRR should probably not apply to process etc. KEPs that do not target production in any way)

/kind bug

(Part of #2348.)

@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Feb 8, 2021
@BenTheElder BenTheElder added the sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. label Feb 8, 2021
@k8s-ci-robot k8s-ci-robot removed the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Feb 8, 2021
@BenTheElder
Copy link
Member Author

BenTheElder commented Feb 8, 2021

assigning per

enhancements/OWNERS_ALIASES

Lines 179 to 183 in e9b84b1

kep-tools-approvers:
- jeremyrickard
- johnbelamaric
- justaugustus
- mrbobbytables

@jeremyrickard
Copy link
Contributor

@BenTheElder thank you for opening this, and I agree with you.

There is some work in flight to refactor much of this stuff, that we are planning to land after enhancement freeze. We'll circle back and review this issue against that and make some additional adjustments to the overall validation as we work through that.

@BenTheElder
Copy link
Member Author

It turns out if it fails to find / parse the file it will just pass :-)
so having kep.yml instead of kep.yaml makes your implementable kep bypass PRR 🙃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants