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

#754 - Validate policy schema #764

Merged
merged 13 commits into from
Mar 28, 2020
Merged

#754 - Validate policy schema #764

merged 13 commits into from
Mar 28, 2020

Conversation

shravanshetty1
Copy link
Contributor

fixes #754

@shravanshetty1 shravanshetty1 changed the title [WIP] #754 - Validate policy schema #754 - Validate policy schema Mar 24, 2020
// normal marshal would cause empty sub structs in
// policy to be non nil.
// TODO This needs to be removed. A simpler way to encode and decode Policy is needed.
func MarshalPolicy(policy v1.ClusterPolicy) []byte {
Copy link
Member

Choose a reason for hiding this comment

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

Agreed, was facing the issue before.

Do you think to change the type in the Policy struct would help?
i.e. add a pointer to Mutation

type Rule struct {
	Mutation         *Mutation         `json:"mutate,omitempty"`
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it would - but this would be a big change and should be tracked in a seperate issue

Copy link
Contributor

Choose a reason for hiding this comment

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

With the suggestion, we are moving from value to pointer semantics.
This was added for optimization, as its easier to manage values than use pointer.
If we plan to remove this optimization, then this needs to updated all places.

// normal marshal would cause empty sub structs in
// policy to be non nil.
// TODO This needs to be removed. A simpler way to encode and decode Policy is needed.
func MarshalPolicy(policy v1.ClusterPolicy) []byte {
Copy link
Contributor

Choose a reason for hiding this comment

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

With the suggestion, we are moving from value to pointer semantics.
This was added for optimization, as its easier to manage values than use pointer.
If we plan to remove this optimization, then this needs to updated all places.

@realshuting
Copy link
Member

@shravanshetty1 Can you please resolve conflicts?

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.

[BUG] Validate policy schema
4 participants