Change to HA-Compatible Status Schemas (#159)

* Change to HA-Compatible Status Schemas

Currently the status schemas used for our resources would
not work if an HA deployment as only one Pod could report
its status at a time. This PR adds a mechanism for multiple
pods to broadcast their handling of a resource.

This fixes #158

Signed-off-by: Max Smythe <smythe@google.com>

* Fix broken CT unit tests

Signed-off-by: Max Smythe <smythe@google.com>

config/crds/config_v1alpha1_config.yaml

@@ -76,15 +76,24 @@ spec:
  type: object
  status:
  properties:
- allFinalizers:
- description: List of Group/Version/Kinds with finalizers
+ byPod:
+ description: List of statuses as seen by individual pods
  items:
  properties:
- group:
- type: string
- kind:
- type: string
- version:
+ allFinalizers:
+ description: List of Group/Version/Kinds with finalizers
+ items:
+ properties:
+ group:
+ type: string
+ kind:
+ type: string
+ version:
+ type: string
+ type: object
+ type: array
+ id:
+ description: a unique identifier for the pod that wrote the status
  type: string
  type: object
  type: array

config/manager/manager.yaml

@@ -55,6 +55,10 @@ spec:
  valueFrom:
  fieldRef:
  fieldPath: metadata.namespace
+ - name: POD_NAME
+ valueFrom:
+ fieldRef:
+ fieldPath: metadata.name
  - name: SECRET_NAME
  value: $(WEBHOOK_SECRET_NAME)
  resources:

deploy/gatekeeper.yaml

@@ -189,6 +189,10 @@ spec:
  fieldRef:
  apiVersion: v1
  fieldPath: metadata.namespace
+ - name: POD_NAME
+ valueFrom:
+ fieldRef:
+ fieldPath: metadata.name
  - name: SECRET_NAME
  value: gatekeeper-webhook-server-secret
  image: quay.io/open-policy-agent/gatekeeper:v3.0.2

pkg/apis/config/v1alpha1/config_types.go

@@ -56,12 +56,19 @@ type SyncOnlyEntry struct {
  Kind string `json:"kind,omitempty"`
 }
 
+type ByPod struct {
+ // a unique identifier for the pod that wrote the status
+ ID string `json:"id,omitempty"`
+ // List of Group/Version/Kinds with finalizers
+ AllFinalizers []GVK `json:"allFinalizers,omitempty"`
+}
+
 // ConfigStatus defines the observed state of Config
 type ConfigStatus struct {
  // Important: Run "make" to regenerate code after modifying this file
 
- // List of Group/Version/Kinds with finalizers
- AllFinalizers []GVK `json:"allFinalizers,omitempty"`
+ // List of statuses as seen by individual pods
+ ByPod []*ByPod `json:"byPod,omitempty"`
 }
 
 func ToAPIGVK(gvk schema.GroupVersionKind) GVK {

pkg/controller/config/config_controller.go

@@ -27,6 +27,7 @@ import (
  configv1alpha1 "github.com/open-policy-agent/gatekeeper/pkg/apis/config/v1alpha1"
  syncc "github.com/open-policy-agent/gatekeeper/pkg/controller/sync"
  "github.com/open-policy-agent/gatekeeper/pkg/target"
+ "github.com/open-policy-agent/gatekeeper/pkg/util"
  "github.com/open-policy-agent/gatekeeper/pkg/watch"
  "k8s.io/apimachinery/pkg/api/errors"
  "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -169,7 +170,8 @@ func (r *ReconcileConfig) Reconcile(request reconcile.Request) (reconcile.Result
  }
  }
  // make sure old finalizers get cleaned up even on restart
- for _, gvk := range instance.Status.AllFinalizers {
+ status := util.GetCfgHAStatus(instance)
+ for _, gvk := range status.AllFinalizers {
  toClean.Add(configv1alpha1.ToGVK(gvk))
  }
 
@@ -191,7 +193,7 @@ func (r *ReconcileConfig) Reconcile(request reconcile.Request) (reconcile.Result
  for i, gvk := range items {
  allFinalizers[i] = configv1alpha1.ToAPIGVK(gvk)
  }
- instance.Status.AllFinalizers = allFinalizers
+ status.AllFinalizers = allFinalizers
  toClean.RemoveSet(newSyncOnly)
  if toClean.Size() > 0 {
  if r.fc != nil {
@@ -215,6 +217,7 @@ func (r *ReconcileConfig) Reconcile(request reconcile.Request) (reconcile.Result
  return reconcile.Result{}, err
  }
 
+ util.SetCfgHAStatus(instance, status)
  if err := r.Update(context.Background(), instance); err != nil {
  return reconcile.Result{}, err
  }
@@ -279,12 +282,14 @@ func (fc *finalizerCleanup) clean() {
  log.Info("could not retrieve config to report removed finalizer")
  }
  var allFinalizers []configv1alpha1.GVK
- for _, v := range instance.Status.AllFinalizers {
+ status := util.GetCfgHAStatus(instance)
+ for _, v := range status.AllFinalizers {
  if configv1alpha1.ToGVK(v) != gvk {
  allFinalizers = append(allFinalizers, v)
  }
  }
- instance.Status.AllFinalizers = allFinalizers
+ status.AllFinalizers = allFinalizers
+ util.SetCfgHAStatus(instance, status)
  if err := fc.c.Update(context.Background(), instance); err != nil {
  log.Info("could not record removed finalizer")
  }

pkg/controller/constraint/constraint_controller.go

@@ -18,8 +18,10 @@ package constraint
 import (
  "context"
  "fmt"
+
  "github.com/go-logr/logr"
  opa "github.com/open-policy-agent/frameworks/constraint/pkg/client"
+ "github.com/open-policy-agent/gatekeeper/pkg/util"
  "k8s.io/apimachinery/pkg/api/errors"
  "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
  "k8s.io/apimachinery/pkg/runtime"
@@ -115,12 +117,22 @@ func (r *ReconcileConstraint) Reconcile(request reconcile.Request) (reconcile.Re
  }
  }
  log.Info("instance will be added", "instance", instance)
- unstructured.RemoveNestedField(instance.Object, "status", "errors")
+ status, err := util.GetHAStatus(instance)
+ if err != nil {
+ return reconcile.Result{}, err
+ }
+ delete(status, "errors")
+ util.SetHAStatus(instance, status)
 
  if _, err := r.opa.AddConstraint(context.Background(), instance); err != nil {
  return reconcile.Result{}, err
  }
- unstructured.SetNestedField(instance.Object, true, "status", "enforced")
+ status, err = util.GetHAStatus(instance)
+ if err != nil {
+ return reconcile.Result{}, err
+ }
+ status["enforced"] = true
+ util.SetHAStatus(instance, status)
  if err := r.Update(context.Background(), instance); err != nil {
  return reconcile.Result{Requeue: true}, nil
  }

pkg/controller/constrainttemplate/constrainttemplate_controller.go

@@ -18,13 +18,14 @@ package constrainttemplate
 import (
  "context"
  "fmt"
- "github.com/open-policy-agent/opa/ast"
  "reflect"
 
  "github.com/open-policy-agent/frameworks/constraint/pkg/apis/templates/v1alpha1"
  opa "github.com/open-policy-agent/frameworks/constraint/pkg/client"
  "github.com/open-policy-agent/gatekeeper/pkg/controller/constraint"
+ "github.com/open-policy-agent/gatekeeper/pkg/util"
  "github.com/open-policy-agent/gatekeeper/pkg/watch"
+ "github.com/open-policy-agent/opa/ast"
  errorpkg "github.com/pkg/errors"
  apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
  "k8s.io/apimachinery/pkg/api/errors"
@@ -134,26 +135,29 @@ func (r *ReconcileConstraintTemplate) Reconcile(request reconcile.Request) (reco
  return reconcile.Result{}, err
  }
 
- instance.Status.Errors = nil
+ status := util.GetCTHAStatus(instance)
+ status.Errors = nil
  crd, err := r.opa.CreateCRD(context.Background(), instance)
  if err != nil {
  var createErr *v1alpha1.CreateCRDError
  if parseErrs, ok := err.(ast.Errors); ok {
  for i := 0; i < len(parseErrs); i++ {
  createErr = &v1alpha1.CreateCRDError{Code: parseErrs[i].Code, Message: parseErrs[i].Message, Location: parseErrs[i].Location.String()}
- instance.Status.Errors = append(instance.Status.Errors, createErr)
+ status.Errors = append(status.Errors, createErr)
  }
  } else {
  createErr = &v1alpha1.CreateCRDError{Code: "create_error", Message: err.Error()}
- instance.Status.Errors = append(instance.Status.Errors, createErr)
+ status.Errors = append(status.Errors, createErr)
  }
 
+ util.SetCTHAStatus(instance, status)
  if updateErr := r.Update(context.Background(), instance); updateErr != nil {
- log.Error(updateErr, "update error", updateErr)
+ log.Error(updateErr, "update error")
  return reconcile.Result{Requeue: true}, nil
  }
  return reconcile.Result{}, nil
  }
+ util.SetCTHAStatus(instance, status)
 
  name := crd.GetName()
  namespace := crd.GetNamespace()
@@ -191,7 +195,9 @@ func (r *ReconcileConstraintTemplate) handleCreate(
  log.Info("loading code into OPA")
  if _, err := r.opa.AddTemplate(context.Background(), instance); err != nil {
  updateErr := &v1alpha1.CreateCRDError{Code: "update_error", Message: fmt.Sprintf("Could not update CRD: %s", err)}
- instance.Status.Errors = append(instance.Status.Errors, updateErr)
+ status := util.GetCTHAStatus(instance)
+ status.Errors = append(status.Errors, updateErr)
+ util.SetCTHAStatus(instance, status)
  if err2 := r.Update(context.Background(), instance); err2 != nil {
  err = errorpkg.Wrap(err, fmt.Sprintf("Could not update status: %s", err2))
  }
@@ -201,11 +207,14 @@ func (r *ReconcileConstraintTemplate) handleCreate(
  if err := r.watcher.AddWatch(makeGvk(instance.Spec.CRD.Spec.Names.Kind)); err != nil {
  return reconcile.Result{}, err
  }
+ // To support HA deployments, only one pod should be able to create CRDs
  log.Info("creating constraint CRD")
  if err := r.Create(context.TODO(), crd); err != nil {
- instance.Status.Errors = []*v1alpha1.CreateCRDError{}
+ status := util.GetCTHAStatus(instance)
+ status.Errors = []*v1alpha1.CreateCRDError{}
  createErr := &v1alpha1.CreateCRDError{Code: "create_error", Message: fmt.Sprintf("Could not create CRD: %s", err)}
- instance.Status.Errors = append(instance.Status.Errors, createErr)
+ status.Errors = append(status.Errors, createErr)
+ util.SetCTHAStatus(instance, status)
  if err2 := r.Update(context.Background(), instance); err2 != nil {
  err = errorpkg.Wrap(err, fmt.Sprintf("Could not update status: %s", err2))
  }
@@ -230,7 +239,9 @@ func (r *ReconcileConstraintTemplate) handleUpdate(
  log.Info("loading constraint code into OPA")
  if _, err := r.opa.AddTemplate(context.Background(), instance); err != nil {
  updateErr := &v1alpha1.CreateCRDError{Code: "update_error", Message: fmt.Sprintf("Could not update CRD: %s", err)}
- instance.Status.Errors = append(instance.Status.Errors, updateErr)
+ status := util.GetCTHAStatus(instance)
+ status.Errors = append(status.Errors, updateErr)
+ util.SetCTHAStatus(instance, status)
  if err2 := r.Update(context.Background(), instance); err2 != nil {
  err = errorpkg.Wrap(err, fmt.Sprintf("Could not update status: %s", err2))
  }

pkg/controller/constrainttemplate/constrainttemplate_controller_test.go

@@ -27,6 +27,7 @@ import (
  opa "github.com/open-policy-agent/frameworks/constraint/pkg/client"
  "github.com/open-policy-agent/frameworks/constraint/pkg/client/drivers/local"
  "github.com/open-policy-agent/gatekeeper/pkg/target"
+ "github.com/open-policy-agent/gatekeeper/pkg/util"
  "github.com/open-policy-agent/gatekeeper/pkg/watch"
  "golang.org/x/net/context"
  admissionv1beta1 "k8s.io/api/admission/v1beta1"
@@ -160,7 +161,11 @@ deny[{"msg": "denied!"}] {
  if err != nil {
  return err
  }
- val, found, err := unstructured.NestedBool(o.Object, "status", "enforced")
+ status, err := util.GetHAStatus(o)
+ if err != nil {
+ return err
+ }
+ val, found, err := unstructured.NestedBool(status, "enforced")
  if err != nil {
  return err
  }
@@ -231,11 +236,12 @@ deny[}}}//invalid//rego
  return err
  }
  if ct.Name == "invalidrego" {
- if len(ct.Status.Errors) != 1 {
+ status := util.GetCTHAStatus(ct)
+ if len(status.Errors) != 1 {
  return errors.New("InvalidRego template should contain 1 parse error")
  } else {
- if ct.Status.Errors[0].Code != "rego_parse_error" {
- return errors.New(fmt.Sprintf("InvalidRego template returning unexpected error %s", ct.Status.Errors[0].Code))
+ if status.Errors[0].Code != "rego_parse_error" {
+ return errors.New(fmt.Sprintf("InvalidRego template returning unexpected error %s", status.Errors[0].Code))
  }
  return nil
  }