datreeio / datree

Prevent Kubernetes misconfigurations from reaching production (again 😤 )! From code to cloud, Datree provides an E2E policy enforcement solution to run automatic checks for rule violations. See our docs: https://hub.datree.io
https://datree.io
Apache License 2.0
6.39k stars 363 forks source link

Validate publish command args before cobra.Run #434

Closed noaabarki closed 1 year ago

noaabarki commented 2 years ago

Describe the solution you'd like

Validate test commands arguments before Run. This pattern will allow us to governance behaviors such as flags/arguments validation in all commands. Additionally, this should make the code easier to understand and maintain.

Requirements Golang basic level.

“How to Implement” suggestion

See issue as a reference.

just-injoey commented 2 years ago

is anyone working on this issue?

adifayer commented 2 years ago

@just-injoey Still up for grabs, would you like to take it? :)

anubha-v-ardhan commented 2 years ago

Hi @adifayer I'm new to golang and would like give this a try. Can I take this?

anubha-v-ardhan commented 2 years ago

Hey @adifayer @noaabarki Can you tell me how we can achieve this, I want to work on this issue.

adifayer commented 2 years ago

@anubha-v-ardhan sure, you got it! Did you check out the issue that we added as a reference?

KiranSatyaRaj commented 1 year ago

Is this still in the works? after going through the referenced issue, I get the gist of what needs to be done, if anyone isn't working on this issue. I can take this up, can you give me your opinion on this @noaabarki?

noaabarki commented 1 year ago

Hi @KiranSatyaRaj, this is still relevant, but we don't want to hijack @anubha-v-ardhan work 😇. @anubha-v-ardhan are you still working on it?

rakshitgondwal commented 1 year ago

Hey! There hasn't been any progress on this for a long time. Can I take this up? @noaabarki

eyarz commented 1 year ago

you're right. go for it!

rakshitgondwal commented 1 year ago

Hey @eyarz! Thanks a lot for assigning me this issue. I just went through the codebase and the other issue mentioned above. I had a question, where do I need to move the logic of validating the arguments in the cmd/publish.main.go?

royhadad commented 1 year ago

I think that the idea is to move the validation into the Args function. but it seems that it already happens there (only 1 argument, which is the path to the PaC.yaml file) https://github.com/datreeio/datree/blob/4cb4d835b8fe8f6b6abefedfad5220b860ec416a/cmd/publish/main.go#LL63C4-L69C5

@noaabarki is this issue still relevant?

noaabarki commented 1 year ago

@rakshitgondwal, @royhadad, both of you are correct. Unfortunately, this issue has already been resolved and is no longer relevant. Sorry for the trouble.