Skip to content

feat: add validation support in marker type#357

Closed
MahmoudShakour wants to merge 1 commit intokubernetes-sigs:mainfrom
MahmoudShakour:add_cel_validation
Closed

feat: add validation support in marker type#357
MahmoudShakour wants to merge 1 commit intokubernetes-sigs:mainfrom
MahmoudShakour:add_cel_validation

Conversation

@MahmoudShakour
Copy link

fixes: #347

wantErr: false,
},
{
name: "validation marker with one value",
Copy link

Choose a reason for hiding this comment

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

Can you add a test for multiple validation markers, and a test for using quotes (e.g. self == "foobar")?

@fsommar
Copy link

fsommar commented Mar 7, 2025

I didn't realize it when writing my feature request, but the fact that each validation is a struct with at least rule and typically also message means that this syntax needs to be updated to support that too

@MahmoudShakour
Copy link
Author

@fsommar
got it. i had that question in my mind but thought you want the default message to be displayed.
what do you think about this format ?

validation="rule:"self == oldSelf",message:"can't be changed"" 

@fsommar
Copy link

fsommar commented Mar 10, 2025

@MahmoudShakour nested quotes is going to be hard to parse for both humans and machines, I think. What about updating the right-hand side to support either strings, like today, or JSON objects? The JSON object suggestion would look something like this:

immutableFoo: |
  string |
  validation={
    "rule": "self == oldSelf",
    "message": "can't be changed"
  }

Another suggestion, where the index is arbitrary as long as it's unique (could be 0 and 1 also):

immutableBar: |
  integer
  validation[immutability].rule="self == oldSelf"
  validation[immutability].message="can't be changed"
  validation[positive].rule="self > 0 || self == -1"
  validation[positive].message="has to be positive or -1"

@a-hilaly
Copy link
Member

a-hilaly commented Apr 1, 2025

Hi @MahmoudShakour - are you still working on this one?

@DhairyaMajmudar
Copy link
Member

@a-hilaly If @MahmoudShakour is okay with it, I’d be happy to take this on since I have priorly worked with Markers logic in PR #424

@a-hilaly
Copy link
Member

a-hilaly commented Apr 1, 2025

please go for it @DhairyaMajmudar - can you please make sure to add @MahmoudShakour as co-author or fork his branch so that we can preserve some of his work.

@a-hilaly a-hilaly closed this Apr 1, 2025
@DhairyaMajmudar
Copy link
Member

Thank you, yeah sure will add him as commit co-author

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.

Implement SimpleSchema CEL validation support

4 participants