Skip to content

Commit

Permalink
Do not trigger a watch of pods for status owner ref (#754)
Browse files Browse the repository at this point in the history
* Do not trigger a watch of pods for status owner ref

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

* Run goimports

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

* Do not use cache for retrieving pod

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

* Unify getPod and add flag to toggle enabling fake get pod

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

* Fix lint errors

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

* Fix nil pointer error

Signed-off-by: Max Smythe <smythe@google.com>
  • Loading branch information
maxsmythe committed Jul 28, 2020
1 parent e189d3f commit e072cf7
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 16 deletions.
11 changes: 3 additions & 8 deletions pkg/controller/constraint/constraint_controller.go
Expand Up @@ -317,14 +317,9 @@ func (r *ReconcileConstraint) Reconcile(request reconcile.Request) (reconcile.Re
}

func (r *ReconcileConstraint) defaultGetPod() (*corev1.Pod, error) {
ns := util.GetNamespace()
name := util.GetPodName()
key := types.NamespacedName{Namespace: ns, Name: name}
pod := &corev1.Pod{}
if err := r.reader.Get(context.TODO(), key, pod); err != nil {
return nil, err
}
return pod, nil
// require injection of GetPod in order to control what client we use to
// guarantee we don't inadvertently create a watch
panic("GetPod must be injected")
}

func (r *ReconcileConstraint) getOrCreatePodStatus(constraint *unstructured.Unstructured) (*constraintstatusv1beta1.ConstraintPodStatus, error) {
Expand Down
Expand Up @@ -485,14 +485,9 @@ func (r *ReconcileConstraintTemplate) handleDelete(
}

func (r *ReconcileConstraintTemplate) defaultGetPod() (*corev1.Pod, error) {
ns := util.GetNamespace()
name := util.GetPodName()
key := types.NamespacedName{Namespace: ns, Name: name}
pod := &corev1.Pod{}
if err := r.Get(context.TODO(), key, pod); err != nil {
return nil, err
}
return pod, nil
// require injection of GetPod in order to control what client we use to
// guarantee we don't inadvertently create a watch
panic("GetPod must be injected")
}

func (r *ReconcileConstraintTemplate) deleteAllStatus(ctName string) error {
Expand Down
75 changes: 75 additions & 0 deletions pkg/controller/controller.go
Expand Up @@ -17,15 +17,29 @@ package controller

import (
"context"
"flag"
"os"
"sync"

opa "github.com/open-policy-agent/frameworks/constraint/pkg/client"
podstatus "github.com/open-policy-agent/gatekeeper/apis/status/v1beta1"
"github.com/open-policy-agent/gatekeeper/pkg/controller/config/process"
"github.com/open-policy-agent/gatekeeper/pkg/readiness"
"github.com/open-policy-agent/gatekeeper/pkg/util"
"github.com/open-policy-agent/gatekeeper/pkg/watch"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
"sigs.k8s.io/controller-runtime/pkg/manager"
)

var (
debugUseFakePod = flag.Bool("debug-use-fake-pod", false, "Use a fake pod name so the Gatekeeper executable can be run outside of Kubernetes")
)

type Injector interface {
InjectOpa(*opa.Client)
InjectWatchManager(*watch.Manager)
Expand Down Expand Up @@ -59,12 +73,73 @@ type Dependencies struct {
ProcessExcluder *process.Excluder
}

type defaultPodGetter struct {
client client.Client
scheme *runtime.Scheme
pod *corev1.Pod
mux sync.RWMutex
}

func (g *defaultPodGetter) GetPod() (*corev1.Pod, error) {
pod := func() *corev1.Pod {
g.mux.RLock()
defer g.mux.RUnlock()
return g.pod
}()
if pod != nil {
return pod.DeepCopy(), nil
}
g.mux.Lock()
defer g.mux.Unlock()
// guard against the race condition where the pod has been retrieved
// between releasing the read lock and acquiring the write lock
if g.pod != nil {
return g.pod.DeepCopy(), nil
}
pod = &corev1.Pod{}
ns := util.GetNamespace()
name := util.GetPodName()
key := types.NamespacedName{Namespace: ns, Name: name}
// use unstructured to avoid inadvertently creating a watch on pods
uPod := &unstructured.Unstructured{}
gvk, err := apiutil.GVKForObject(pod, g.scheme)
if err != nil {
return nil, err
}
uPod.SetGroupVersionKind(gvk)
if err := g.client.Get(context.TODO(), key, uPod); err != nil {
return nil, err
}
if err := g.scheme.Convert(uPod, pod, nil); err != nil {
return nil, err
}
g.pod = pod
return pod.DeepCopy(), nil
}

// AddToManager adds all Controllers to the Manager
func AddToManager(m manager.Manager, deps Dependencies) error {
// Reset cache on start - this is to allow for the future possibility that the OPA cache is stored remotely
if err := deps.Opa.Reset(context.Background()); err != nil {
return err
}
if deps.GetPod == nil {
podGetter := &defaultPodGetter{
scheme: m.GetScheme(),
client: m.GetClient(),
}
deps.GetPod = podGetter.GetPod
}
if *debugUseFakePod {
os.Setenv("POD_NAME", "no-pod")
podstatus.DisablePodOwnership()
fakePodGetter := func() (*corev1.Pod, error) {
pod := &corev1.Pod{}
pod.Name = util.GetPodName()
return pod, nil
}
deps.GetPod = fakePodGetter
}
for _, a := range Injectors {
a.InjectOpa(deps.Opa)
a.InjectWatchManager(deps.WatchManger)
Expand Down

0 comments on commit e072cf7

Please sign in to comment.