-
Notifications
You must be signed in to change notification settings - Fork 762
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
chore: adding enforcement point status, vapgeneratestatus #3686
Changes from 6 commits
a492dcd
c72db95
8c27363
ceedd2d
d65d3c3
8183975
0e40ed2
0238c73
a4f17a3
d67ef29
ae48233
8ef8278
b25ca53
a935efd
c33b601
5dff00d
1afa5ac
9727998
45a53f7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,11 +29,18 @@ import ( | |
// ConstraintTemplatePodStatusStatus defines the observed state of ConstraintTemplatePodStatus. | ||
type ConstraintTemplatePodStatusStatus struct { | ||
// Important: Run "make" to regenerate code after modifying this file | ||
ID string `json:"id,omitempty"` | ||
TemplateUID types.UID `json:"templateUID,omitempty"` | ||
Operations []string `json:"operations,omitempty"` | ||
ObservedGeneration int64 `json:"observedGeneration,omitempty"` | ||
Errors []*templatesv1beta1.CreateCRDError `json:"errors,omitempty"` | ||
ID string `json:"id,omitempty"` | ||
TemplateUID types.UID `json:"templateUID,omitempty"` | ||
Operations []string `json:"operations,omitempty"` | ||
ObservedGeneration int64 `json:"observedGeneration,omitempty"` | ||
Errors []*templatesv1beta1.CreateCRDError `json:"errors,omitempty"` | ||
VAPGenerationStatus VAPGenerationStatus `json:"vapGenerationStatus,omitempty"` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can remove this if we do not want status on CT for generation updates. Lmk your thoughts. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it makes sense to report status for CT objects... they do map directly to a VAP object. |
||
} | ||
|
||
// VAPGenerationStatus represents the status of VAP generation. | ||
type VAPGenerationStatus struct { | ||
Generated bool `json:"generated,omitempty"` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment about bool field vs. string field. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated to string field. |
||
Warning string `json:"warning,omitempty"` | ||
} | ||
|
||
// +kubebuilder:object:root=true | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
I think it was a mistake to have an
enforced
boolean state with constraints. And with VAP, we cannot make guarantees as to whether they are enforced b/c there is no status reporting, so we cannot know if the VAP object has been ingested or whether there are any errors. Maybe we want something like a string field calledStatus
?There we can put a string like "GENERATED", "ERROR", etc.
Of course, if we call it "status", we should rename the parent field to avoid stutter.
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.
how about
code
-generated/error/...
?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.
"code" seems a bit ambiguous given we are also dealing with "code" in the sense of CEL code.
"state"?
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.
I like
state
, updated the name tostate
.