add ownerrefs to SSCS

pkg/cmd/server/bootstrappolicy/controller_policy.go

@@ -229,7 +229,7 @@ func init() {
 		ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + InfraServiceServingCertServiceAccountName},
 		Rules: []rbac.PolicyRule{
 			rbac.NewRule("list", "watch", "update").Groups(kapiGroup).Resources("services").RuleOrDie(),
-			rbac.NewRule("get", "list", "watch", "create", "update").Groups(kapiGroup).Resources("secrets").RuleOrDie(),
+			rbac.NewRule("get", "list", "watch", "create", "update", "delete").Groups(kapiGroup).Resources("secrets").RuleOrDie(),
 			eventsRule(),
 		},
 	})

pkg/controller/ownerref.go

@@ -0,0 +1,60 @@
+package controller
+
+import (
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	kapihelper "k8s.io/kubernetes/pkg/api/helper"
+)
+
+// EnsureOwnerRef adds the ownerref if needed. Removes ownerrefs with conflicting UIDs.
+// Returns true if the input is mutated.
+func EnsureOwnerRef(metadata metav1.Object, newOwnerRef metav1.OwnerReference) bool {
+	foundButNotEqual := false
+	for _, existingOwnerRef := range metadata.GetOwnerReferences() {
+		if existingOwnerRef.APIVersion == newOwnerRef.APIVersion &&
+			existingOwnerRef.Kind == newOwnerRef.Kind &&
+			existingOwnerRef.Name == newOwnerRef.Name {
+
+			// if we're completely the same, there's nothing to do
+			if kapihelper.Semantic.DeepEqual(existingOwnerRef, newOwnerRef) {
+				return false
+			}
+
+			foundButNotEqual = true
+			break
+		}
+	}
+
+	// if we weren't found, then we just need to add ourselves
+	if !foundButNotEqual {
+		metadata.SetOwnerReferences(append(metadata.GetOwnerReferences(), newOwnerRef))
+		return true
+	}
+
+	// if we need to remove an existing ownerRef, just do the easy thing and build it back from scratch
+	newOwnerRefs := []metav1.OwnerReference{newOwnerRef}
+	for i := range metadata.GetOwnerReferences() {
+		existingOwnerRef := metadata.GetOwnerReferences()[i]
+		if existingOwnerRef.APIVersion == newOwnerRef.APIVersion &&
+			existingOwnerRef.Kind == newOwnerRef.Kind &&
+			existingOwnerRef.Name == newOwnerRef.Name {
+			continue
+		}
+		newOwnerRefs = append(newOwnerRefs, existingOwnerRef)
+	}
+	metadata.SetOwnerReferences(newOwnerRefs)
+	return true
+}
+
+// HasOwnerRef checks to see if an object has a particular owner.  It is not opinionated about
+// the bool fields
+func HasOwnerRef(metadata metav1.Object, needle metav1.OwnerReference) bool {
+	for _, existingOwnerRef := range metadata.GetOwnerReferences() {
+		if existingOwnerRef.APIVersion == needle.APIVersion &&
+			existingOwnerRef.Kind == needle.Kind &&
+			existingOwnerRef.Name == needle.Name &&
+			existingOwnerRef.UID == needle.UID {
+			return true
+		}
+	}
+	return false
+}

pkg/controller/ownerref_test.go

@@ -0,0 +1,221 @@
+package controller
+
+import (
+	"testing"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+	kapihelper "k8s.io/kubernetes/pkg/api/helper"
+)
+
+func TestEnsureOwnerRef(t *testing.T) {
+	tests := []struct {
+		name           string
+		obj            *metav1.ObjectMeta
+		newOwnerRef    metav1.OwnerReference
+		expectedOwners []metav1.OwnerReference
+		expectedReturn bool
+	}{
+		{
+			name: "empty",
+			obj:  &metav1.ObjectMeta{},
+			newOwnerRef: metav1.OwnerReference{
+				APIVersion: "v1", Kind: "Foo", Name: "the-name", UID: types.UID("uid"),
+			},
+			expectedOwners: []metav1.OwnerReference{
+				{APIVersion: "v1", Kind: "Foo", Name: "the-name", UID: types.UID("uid")},
+			},
+			expectedReturn: true,
+		},
+		{
+			name: "add",
+			obj: &metav1.ObjectMeta{
+				OwnerReferences: []metav1.OwnerReference{
+					{APIVersion: "v1", Kind: "Foo", Name: "the-other", UID: types.UID("other-uid")},
+				},
+			},
+			newOwnerRef: metav1.OwnerReference{
+				APIVersion: "v1", Kind: "Foo", Name: "the-name", UID: types.UID("uid"),
+			},
+			expectedOwners: []metav1.OwnerReference{
+				{APIVersion: "v1", Kind: "Foo", Name: "the-other", UID: types.UID("other-uid")},
+				{APIVersion: "v1", Kind: "Foo", Name: "the-name", UID: types.UID("uid")},
+			},
+			expectedReturn: true,
+		},
+		{
+			name: "skip",
+			obj: &metav1.ObjectMeta{
+				OwnerReferences: []metav1.OwnerReference{
+					{APIVersion: "v1", Kind: "Foo", Name: "the-name", UID: types.UID("uid")},
+					{APIVersion: "v1", Kind: "Foo", Name: "the-other", UID: types.UID("other-uid")},
+				},
+			},
+			newOwnerRef: metav1.OwnerReference{
+				APIVersion: "v1", Kind: "Foo", Name: "the-name", UID: types.UID("uid"),
+			},
+			expectedOwners: []metav1.OwnerReference{
+				{APIVersion: "v1", Kind: "Foo", Name: "the-name", UID: types.UID("uid")},
+				{APIVersion: "v1", Kind: "Foo", Name: "the-other", UID: types.UID("other-uid")},
+			},
+			expectedReturn: false,
+		},
+		{
+			name: "replace on uid",
+			obj: &metav1.ObjectMeta{
+				OwnerReferences: []metav1.OwnerReference{
+					{APIVersion: "v1", Kind: "Foo", Name: "the-other", UID: types.UID("other-uid")},
+					{APIVersion: "v1", Kind: "Foo", Name: "the-name", UID: types.UID("bad-uid")},
+				},
+			},
+			newOwnerRef: metav1.OwnerReference{
+				APIVersion: "v1", Kind: "Foo", Name: "the-name", UID: types.UID("uid"),
+			},
+			expectedOwners: []metav1.OwnerReference{
+				{APIVersion: "v1", Kind: "Foo", Name: "the-name", UID: types.UID("uid")},
+				{APIVersion: "v1", Kind: "Foo", Name: "the-other", UID: types.UID("other-uid")},
+			},
+			expectedReturn: true,
+		},
+		{
+			name: "preserve controller",
+			obj: &metav1.ObjectMeta{
+				OwnerReferences: []metav1.OwnerReference{
+					{APIVersion: "v1", Kind: "Foo", Name: "the-other", UID: types.UID("other-uid")},
+					{APIVersion: "v1", Kind: "Foo", Name: "the-name", UID: types.UID("uid")},
+				},
+			},
+			newOwnerRef: metav1.OwnerReference{
+				APIVersion: "v1", Kind: "Foo", Name: "the-name", UID: types.UID("uid"), Controller: boolPtr(true),
+			},
+			expectedOwners: []metav1.OwnerReference{
+				{APIVersion: "v1", Kind: "Foo", Name: "the-name", UID: types.UID("uid"), Controller: boolPtr(true)},
+				{APIVersion: "v1", Kind: "Foo", Name: "the-other", UID: types.UID("other-uid")},
+			},
+			expectedReturn: true,
+		},
+		{
+			name: "preserve block",
+			obj: &metav1.ObjectMeta{
+				OwnerReferences: []metav1.OwnerReference{
+					{APIVersion: "v1", Kind: "Foo", Name: "the-other", UID: types.UID("other-uid")},
+					{APIVersion: "v1", Kind: "Foo", Name: "the-name", UID: types.UID("uid")},
+				},
+			},
+			newOwnerRef: metav1.OwnerReference{
+				APIVersion: "v1", Kind: "Foo", Name: "the-name", UID: types.UID("uid"), BlockOwnerDeletion: boolPtr(false),
+			},
+			expectedOwners: []metav1.OwnerReference{
+				{APIVersion: "v1", Kind: "Foo", Name: "the-name", UID: types.UID("uid"), BlockOwnerDeletion: boolPtr(false)},
+				{APIVersion: "v1", Kind: "Foo", Name: "the-other", UID: types.UID("other-uid")},
+			},
+			expectedReturn: true,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			actualReturn := EnsureOwnerRef(tc.obj, tc.newOwnerRef)
+			if tc.expectedReturn != actualReturn {
+				t.Errorf("expected %v, got %v", tc.expectedReturn, actualReturn)
+				return
+			}
+			if !kapihelper.Semantic.DeepEqual(tc.expectedOwners, tc.obj.OwnerReferences) {
+				t.Errorf("expected %v, got %v", tc.expectedOwners, tc.obj.OwnerReferences)
+				return
+			}
+		})
+	}
+}
+
+func boolPtr(in bool) *bool {
+	return &in
+}
+
+func TestHasOwnerRef(t *testing.T) {
+	tests := []struct {
+		name     string
+		haystack *metav1.ObjectMeta
+		needle   metav1.OwnerReference
+		expected bool
+	}{
+		{
+			name:     "empty",
+			haystack: &metav1.ObjectMeta{},
+			needle: metav1.OwnerReference{
+				APIVersion: "v1", Kind: "Foo", Name: "the-name", UID: types.UID("uid"),
+			},
+			expected: false,
+		},
+		{
+			name: "exact",
+			haystack: &metav1.ObjectMeta{
+				OwnerReferences: []metav1.OwnerReference{{
+					APIVersion: "v1", Kind: "Foo", Name: "the-name", UID: types.UID("uid"),
+				}},
+			},
+			needle: metav1.OwnerReference{
+				APIVersion: "v1", Kind: "Foo", Name: "the-name", UID: types.UID("uid"),
+			},
+			expected: true,
+		},
+		{
+			name: "not uid",
+			haystack: &metav1.ObjectMeta{
+				OwnerReferences: []metav1.OwnerReference{{
+					APIVersion: "v1", Kind: "Foo", Name: "the-name",
+				}},
+			},
+			needle: metav1.OwnerReference{
+				APIVersion: "v1", Kind: "Foo", Name: "the-name", UID: types.UID("uid"),
+			},
+			expected: false,
+		},
+		{
+			name: "ignored controller",
+			haystack: &metav1.ObjectMeta{
+				OwnerReferences: []metav1.OwnerReference{{
+					APIVersion: "v1", Kind: "Foo", Name: "the-name", UID: types.UID("uid"),
+				}},
+			},
+			needle: metav1.OwnerReference{
+				APIVersion: "v1", Kind: "Foo", Name: "the-name", UID: types.UID("uid"), Controller: boolPtr(true),
+			},
+			expected: true,
+		},
+		{
+			name: "ignored block",
+			haystack: &metav1.ObjectMeta{
+				OwnerReferences: []metav1.OwnerReference{{
+					APIVersion: "v1", Kind: "Foo", Name: "the-name", UID: types.UID("uid"),
+				}},
+			},
+			needle: metav1.OwnerReference{
+				APIVersion: "v1", Kind: "Foo", Name: "the-name", UID: types.UID("uid"), BlockOwnerDeletion: boolPtr(false),
+			},
+			expected: true,
+		},
+		{
+			name: "ignored both",
+			haystack: &metav1.ObjectMeta{
+				OwnerReferences: []metav1.OwnerReference{{
+					APIVersion: "v1", Kind: "Foo", Name: "the-name", UID: types.UID("uid"), Controller: boolPtr(false),
+				}},
+			},
+			needle: metav1.OwnerReference{
+				APIVersion: "v1", Kind: "Foo", Name: "the-name", UID: types.UID("uid"), BlockOwnerDeletion: boolPtr(false),
+			},
+			expected: true,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			actual := HasOwnerRef(tc.haystack, tc.needle)
+			if tc.expected != actual {
+				t.Errorf("expected %v, got %v", tc.expected, actual)
+				return
+			}
+		})
+	}
+}

pkg/service/controller/servingcert/secret_creating_controller.go

@@ -23,6 +23,7 @@ import (
 
 	"github.com/openshift/origin/pkg/cmd/server/crypto"
 	"github.com/openshift/origin/pkg/cmd/server/crypto/extensions"
+	ocontroller "github.com/openshift/origin/pkg/controller"
 )
 
 const (
@@ -263,6 +264,7 @@ func (sc *ServiceServingCertController) syncService(key string) error {
 			v1.TLSPrivateKeyKey: keyBytes,
 		},
 	}
+	ocontroller.EnsureOwnerRef(secret, ownerRef(service))
 
 	_, err = sc.secretClient.Secrets(service.Namespace).Create(secret)
 	if err != nil && !kapierrors.IsAlreadyExists(err) {
@@ -345,3 +347,12 @@ func (sc *ServiceServingCertController) requiresCertGeneration(service *v1.Servi
 	}
 	return true
 }
+
+func ownerRef(service *v1.Service) metav1.OwnerReference {
+	return metav1.OwnerReference{
+		APIVersion: "v1",
+		Kind:       "Service",
+		Name:       service.Name,
+		UID:        service.UID,
+	}
+}

pkg/service/controller/servingcert/secret_creating_controller_test.go

@@ -11,10 +11,12 @@ import (
 	"time"
 
 	kapierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/watch"
 	clientgotesting "k8s.io/client-go/testing"
+	kapihelper "k8s.io/kubernetes/pkg/api/helper"
 	"k8s.io/kubernetes/pkg/api/v1"
 	"k8s.io/kubernetes/pkg/client/clientset_generated/clientset/fake"
 	informers "k8s.io/kubernetes/pkg/client/informers/informers_generated/externalversions"
@@ -512,6 +514,7 @@ func TestRecreateSecretControllerFlow(t *testing.T) {
 	serviceUID := "some-uid"
 	expectedServiceAnnotations := map[string]string{ServingCertSecretAnnotation: expectedSecretName, ServingCertCreatedByAnnotation: caName}
 	expectedSecretAnnotations := map[string]string{ServiceUIDAnnotation: serviceUID, ServiceNameAnnotation: serviceName}
+	expectedOwnerRef := []metav1.OwnerReference{{APIVersion: "v1", Kind: "Service", Name: serviceName, UID: types.UID(serviceUID)}}
 	namespace := "ns"
 
 	serviceToAdd := &v1.Service{}
@@ -553,6 +556,10 @@ func TestRecreateSecretControllerFlow(t *testing.T) {
 				t.Errorf("expected %v, got %v", expectedSecretAnnotations, newSecret.Annotations)
 				continue
 			}
+			if !kapihelper.Semantic.DeepEqual(expectedOwnerRef, newSecret.OwnerReferences) {
+				t.Errorf("expected %v, got %v", expectedOwnerRef, newSecret.OwnerReferences)
+				continue
+			}
 
 			checkGeneratedCertificate(t, newSecret.Data["tls.crt"], serviceToAdd)
 			foundSecret = true

pkg/service/controller/servingcert/secret_updating_controller.go

@@ -19,6 +19,7 @@ import (
 
 	"github.com/openshift/origin/pkg/cmd/server/crypto"
 	"github.com/openshift/origin/pkg/cmd/server/crypto/extensions"
+	ocontroller "github.com/openshift/origin/pkg/controller"
 )
 
 // ServiceServingCertUpdateController is responsible for synchronizing Service objects stored
@@ -199,6 +200,7 @@ func (sc *ServiceServingCertUpdateController) syncSecret(key string) error {
 	if err != nil {
 		return err
 	}
+	ocontroller.EnsureOwnerRef(secret, ownerRef(service))
 
 	_, err = sc.secretClient.Secrets(secret.Namespace).Update(secret)
 	return err
@@ -226,6 +228,12 @@ func (sc *ServiceServingCertUpdateController) requiresRegeneration(secret *v1.Se
 		return false, nil
 	}
 
+	// if we don't have an ownerref, just go ahead and regenerate.  It's easier than writing a
+	// secondary logic flow.
+	if !ocontroller.HasOwnerRef(secret, ownerRef(service)) {
+		return true, service
+	}
+
 	// if we don't have the annotation for expiry, just go ahead and regenerate.  It's easier than writing a
 	// secondary logic flow that creates the expiry dates
 	expiryString, ok := secret.Annotations[ServingCertExpiryAnnotation]