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

chore: adding enforcement point status, vapgeneratestatus #3686

Merged
Merged
Changes from 18 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions apis/status/v1beta1/constraintpodstatus_types.go
Original file line number Diff line number Diff line change
@@ -39,11 +39,12 @@ type ConstraintPodStatusStatus struct {
// Storing the constraint UID allows us to detect drift, such as
// when a constraint has been recreated after its CRD was deleted
// out from under it, interrupting the watch
ConstraintUID types.UID `json:"constraintUID,omitempty"`
Operations []string `json:"operations,omitempty"`
Enforced bool `json:"enforced,omitempty"`
Errors []Error `json:"errors,omitempty"`
ObservedGeneration int64 `json:"observedGeneration,omitempty"`
ConstraintUID types.UID `json:"constraintUID,omitempty"`
Operations []string `json:"operations,omitempty"`
Enforced bool `json:"enforced,omitempty"`
Errors []Error `json:"errors,omitempty"`
ObservedGeneration int64 `json:"observedGeneration,omitempty"`
EnforcementPointsStatus []EnforcementPointStatus `json:"enforcementPointsStatus,omitempty"`
}

// Error represents a single error caught while adding a constraint to engine.
@@ -53,6 +54,14 @@ type Error struct {
Location string `json:"location,omitempty"`
}

// EnforcementPointStatus represents the status of a single enforcement point.
type EnforcementPointStatus struct {
EnforcementPoint string `json:"enforcementPoint"`
State string `json:"state"`
Message string `json:"message,omitempty"`
ObservedGeneration int64 `json:"observedGeneration,omitempty"`
}

// +kubebuilder:object:root=true
// +kubebuilder:resource:scope=Namespaced

18 changes: 13 additions & 5 deletions apis/status/v1beta1/constrainttemplatepodstatus_types.go
Original file line number Diff line number Diff line change
@@ -29,11 +29,19 @@ 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"`
}

// VAPGenerationStatus represents the status of VAP generation.
type VAPGenerationStatus struct {
State string `json:"state,omitempty"`
ObservedGeneration int64 `json:"observedGeneration,omitempty"`
Warning string `json:"warning,omitempty"`
}

// +kubebuilder:object:root=true
40 changes: 40 additions & 0 deletions apis/status/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -48,6 +48,25 @@ spec:
type: string
enforced:
type: boolean
enforcementPointsStatus:
items:
description: EnforcementPointStatus represents the status of a single
enforcement point.
properties:
enforcementPoint:
type: string
message:
type: string
observedGeneration:
format: int64
type: integer
state:
type: string
required:
- enforcementPoint
- state
type: object
type: array
errors:
items:
description: Error represents a single error caught while adding
Original file line number Diff line number Diff line change
@@ -74,6 +74,17 @@ spec:
don't ONLY use UUIDs, this is an alias to string. Being a type captures
intent and helps make sure that UIDs and names do not get conflated.
type: string
vapGenerationStatus:
description: VAPGenerationStatus represents the status of VAP generation.
properties:
observedGeneration:
format: int64
type: integer
state:
type: string
warning:
type: string
type: object
type: object
type: object
served: true
Original file line number Diff line number Diff line change
@@ -50,6 +50,24 @@ spec:
type: string
enforced:
type: boolean
enforcementPointsStatus:
items:
description: EnforcementPointStatus represents the status of a single enforcement point.
properties:
enforcementPoint:
type: string
message:
type: string
observedGeneration:
format: int64
type: integer
state:
type: string
required:
- enforcementPoint
- state
type: object
type: array
errors:
items:
description: Error represents a single error caught while adding a constraint to engine.
Original file line number Diff line number Diff line change
@@ -73,6 +73,17 @@ spec:
don't ONLY use UUIDs, this is an alias to string. Being a type captures
intent and helps make sure that UIDs and names do not get conflated.
type: string
vapGenerationStatus:
description: VAPGenerationStatus represents the status of VAP generation.
properties:
observedGeneration:
format: int64
type: integer
state:
type: string
warning:
type: string
type: object
type: object
type: object
served: true
29 changes: 29 additions & 0 deletions manifest_staging/deploy/gatekeeper.yaml
Original file line number Diff line number Diff line change
@@ -2660,6 +2660,24 @@ spec:
type: string
enforced:
type: boolean
enforcementPointsStatus:
items:
description: EnforcementPointStatus represents the status of a single enforcement point.
properties:
enforcementPoint:
type: string
message:
type: string
observedGeneration:
format: int64
type: integer
state:
type: string
required:
- enforcementPoint
- state
type: object
type: array
errors:
items:
description: Error represents a single error caught while adding a constraint to engine.
@@ -2763,6 +2781,17 @@ spec:
don't ONLY use UUIDs, this is an alias to string. Being a type captures
intent and helps make sure that UIDs and names do not get conflated.
type: string
vapGenerationStatus:
description: VAPGenerationStatus represents the status of VAP generation.
properties:
observedGeneration:
format: int64
type: integer
state:
type: string
warning:
type: string
type: object
type: object
type: object
served: true
61 changes: 44 additions & 17 deletions pkg/controller/constraint/constraint_controller.go
Original file line number Diff line number Diff line change
@@ -65,6 +65,9 @@ import (

const (
BlockVAPBGenerationUntilAnnotation = "gatekeeper.sh/block-vapb-generation-until"
ErrGenerateVAPBState = "error"
GeneratedVAPBState = "generated"
WaitVAPBState = "waiting"
)

var (
@@ -489,39 +492,39 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction
return noDelay, err
}

generateVAPB, VAPEnforcementActions, err := shouldGenerateVAPB(*DefaultGenerateVAPB, enforcementAction, instance)
shouldGenerateVAPB, VAPEnforcementActions, err := shouldGenerateVAPB(*DefaultGenerateVAPB, enforcementAction, instance)
if err != nil {
log.Error(err, "could not determine if ValidatingAdmissionPolicyBinding should be generated")
return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, "could not determine if ValidatingAdmissionPolicyBinding should be generated")
}
isAPIEnabled := false
var groupVersion *schema.GroupVersion
if generateVAPB {
if shouldGenerateVAPB {
isAPIEnabled, groupVersion = transform.IsVapAPIEnabled(&log)
}
if generateVAPB {
if shouldGenerateVAPB {
if !isAPIEnabled {
log.Error(ErrValidatingAdmissionPolicyAPIDisabled, "Cannot generate ValidatingAdmissionPolicyBinding", "constraint", instance.GetName())
_ = r.reportErrorOnConstraintStatus(ctx, status, ErrValidatingAdmissionPolicyAPIDisabled, "cannot generate ValidatingAdmissionPolicyBinding")
generateVAPB = false
status.Status.Errors = append(status.Status.Errors, constraintstatusv1beta1.Error{Message: fmt.Sprintf("cannot generate ValidatingAdmissionPolicyBinding: %s", ErrValidatingAdmissionPolicyAPIDisabled)})
shouldGenerateVAPB = false
} else {
unversionedCT := &templates.ConstraintTemplate{}
if err := r.scheme.Convert(ct, unversionedCT, nil); err != nil {
return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, "could not convert ConstraintTemplate to unversioned")
}
hasVAP, err := ShouldGenerateVAP(unversionedCT)
switch {
case errors.Is(err, celSchema.ErrCodeNotDefined):
// TODO jgabani: follow up with enforcementPointStatus field under bypod to not swallow this error.
generateVAPB = false
case errors.Is(err, celSchema.ErrCELEngineMissing):
updateEnforcementPointStatus(status, util.VAPEnforcementPoint, ErrGenerateVAPBState, err.Error(), instance.GetGeneration())
shouldGenerateVAPB = false
case err != nil:
log.Error(err, "could not determine if ConstraintTemplate is configured to generate ValidatingAdmissionPolicy", "constraint", instance.GetName(), "constraint_template", unversionedCT.GetName())
_ = r.reportErrorOnConstraintStatus(ctx, status, err, "could not determine if ConstraintTemplate is configured to generate ValidatingAdmissionPolicy")
generateVAPB = false
status.Status.Errors = append(status.Status.Errors, constraintstatusv1beta1.Error{Message: fmt.Sprintf("could not determine if ConstraintTemplate is configured to generate ValidatingAdmissionPolicy: %s", err)})
shouldGenerateVAPB = false
case !hasVAP:
log.Error(ErrVAPConditionsNotSatisfied, "Cannot generate ValidatingAdmissionPolicyBinding", "constraint", instance.GetName(), "constraint_template", unversionedCT.GetName())
_ = r.reportErrorOnConstraintStatus(ctx, status, ErrVAPConditionsNotSatisfied, "Cannot generate ValidatingAdmissionPolicyBinding")
generateVAPB = false
status.Status.Errors = append(status.Status.Errors, constraintstatusv1beta1.Error{Message: fmt.Sprintf("cannot generate ValidatingAdmissionPolicyBinding: %s", ErrVAPConditionsNotSatisfied)})
shouldGenerateVAPB = false
default:
// reconcile for vapb generation if annotation is not set
if ct.Annotations == nil || ct.Annotations[BlockVAPBGenerationUntilAnnotation] == "" {
@@ -535,15 +538,17 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction
return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, "could not parse timestamp")
}
if t.After(time.Now()) {
return time.Until(t), nil
wait := time.Until(t)
updateEnforcementPointStatus(status, util.VAPEnforcementPoint, WaitVAPBState, fmt.Sprintf("waiting for %s before generating ValidatingAdmissionPolicyBinding to make sure api-server has cached constraint CRD", wait), instance.GetGeneration())
return wait, r.writer.Update(ctx, status)
}
}
}
}

r.log.Info("constraint controller", "generateVAPB", generateVAPB)
r.log.Info("constraint controller", "generateVAPB", shouldGenerateVAPB)
// generate vapbinding resources
if generateVAPB && groupVersion != nil {
if shouldGenerateVAPB && groupVersion != nil {
currentVapBinding, err := vapBindingForVersion(*groupVersion)
if err != nil {
return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, "could not get ValidatingAdmissionPolicyBinding API version")
@@ -581,10 +586,11 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction
return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, fmt.Sprintf("could not update ValidatingAdmissionPolicyBinding: %s", vapBindingName))
}
}
updateEnforcementPointStatus(status, util.VAPEnforcementPoint, GeneratedVAPBState, "", instance.GetGeneration())
}
// do not generate vapbinding resources
// remove if exists
if !generateVAPB && groupVersion != nil {
if !shouldGenerateVAPB && groupVersion != nil {
currentVapBinding, err := vapBindingForVersion(*groupVersion)
if err != nil {
return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, "could not get ValidatingAdmissionPolicyBinding API version")
@@ -602,9 +608,10 @@ func (r *ReconcileConstraint) manageVAPB(ctx context.Context, enforcementAction
if err := r.writer.Delete(ctx, currentVapBinding); err != nil {
return noDelay, r.reportErrorOnConstraintStatus(ctx, status, err, fmt.Sprintf("could not delete ValidatingAdmissionPolicyBinding: %s", vapBindingName))
}
cleanEnforcementPointStatus(status, util.VAPEnforcementPoint)
}
}
return noDelay, nil
return noDelay, r.writer.Update(ctx, status)
}

func NewConstraintsCache() *ConstraintsCache {
@@ -726,3 +733,23 @@ func v1beta1ToV1(v1beta1Obj *admissionregistrationv1beta1.ValidatingAdmissionPol

return obj, nil
}

func updateEnforcementPointStatus(status *constraintstatusv1beta1.ConstraintPodStatus, enforcementPoint string, state string, message string, observedGeneration int64) {
vapEnforcementPointStatus := constraintstatusv1beta1.EnforcementPointStatus{EnforcementPoint: enforcementPoint, State: state, ObservedGeneration: observedGeneration, Message: message}
for i, ep := range status.Status.EnforcementPointsStatus {
if ep.EnforcementPoint == enforcementPoint {
status.Status.EnforcementPointsStatus[i] = vapEnforcementPointStatus
Copy link
Member

Choose a reason for hiding this comment

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

enforcementPoint can be more than just vap enforcement point right? this var name vapEnforcementPointStatus makes it look like its just for the vap EP.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated the variable to make it generic.

return
}
}
status.Status.EnforcementPointsStatus = append(status.Status.EnforcementPointsStatus, vapEnforcementPointStatus)
}

func cleanEnforcementPointStatus(status *constraintstatusv1beta1.ConstraintPodStatus, enforcementPoint string) {
for i, ep := range status.Status.EnforcementPointsStatus {
if ep.EnforcementPoint == enforcementPoint {
status.Status.EnforcementPointsStatus = append(status.Status.EnforcementPointsStatus[:i], status.Status.EnforcementPointsStatus[i+1:]...)
return
}
}
}
Loading