Merge pull request #16517 from enj/enj/i/kube_bootstrap_roles

Automatic merge from submit-queue Bootstrap Kube namespaced roles and bindings This change brings in the upstream namespaced roles that are used by controllers. These will never conflict with the openshift ones since they are required to be in namespaces prefixed with "kube-". Signed-off-by: Monis Khan <[email protected]>
openshift · Sep 27, 2017 · 56bc677 · 56bc677
2 parents 848776c + f42b2d7
commit 56bc677
Show file tree

Hide file tree

Showing 20 changed files with 8,042 additions and 144 deletions.
diff --git a/contrib/completions/bash/oadm b/contrib/completions/bash/oadm
@@ -1657,8 +1657,6 @@ _oadm_create-bootstrap-policy-file()
     flags_with_completion+=("--filename")
     flags_completion+=("_filedir")
     local_nonpersistent_flags+=("--filename=")
-    flags+=("--openshift-namespace=")
-    local_nonpersistent_flags+=("--openshift-namespace=")
     flags+=("--as=")
     flags+=("--as-group=")
     flags+=("--certificate-authority=")

diff --git a/contrib/completions/bash/oc b/contrib/completions/bash/oc
@@ -1822,8 +1822,6 @@ _oc_adm_create-bootstrap-policy-file()
     flags_with_completion+=("--filename")
     flags_completion+=("_filedir")
     local_nonpersistent_flags+=("--filename=")
-    flags+=("--openshift-namespace=")
-    local_nonpersistent_flags+=("--openshift-namespace=")
     flags+=("--as=")
     flags+=("--as-group=")
     flags+=("--certificate-authority=")

diff --git a/contrib/completions/bash/openshift b/contrib/completions/bash/openshift
@@ -1657,8 +1657,6 @@ _openshift_admin_create-bootstrap-policy-file()
     flags_with_completion+=("--filename")
     flags_completion+=("_filedir")
     local_nonpersistent_flags+=("--filename=")
-    flags+=("--openshift-namespace=")
-    local_nonpersistent_flags+=("--openshift-namespace=")
     flags+=("--as=")
     flags+=("--as-group=")
     flags+=("--certificate-authority=")
@@ -7082,8 +7080,6 @@ _openshift_cli_adm_create-bootstrap-policy-file()
     flags_with_completion+=("--filename")
     flags_completion+=("_filedir")
     local_nonpersistent_flags+=("--filename=")
-    flags+=("--openshift-namespace=")
-    local_nonpersistent_flags+=("--openshift-namespace=")
     flags+=("--as=")
     flags+=("--as-group=")
     flags+=("--certificate-authority=")

diff --git a/contrib/completions/zsh/oadm b/contrib/completions/zsh/oadm
@@ -1806,8 +1806,6 @@ _oadm_create-bootstrap-policy-file()
     flags_with_completion+=("--filename")
     flags_completion+=("_filedir")
     local_nonpersistent_flags+=("--filename=")
-    flags+=("--openshift-namespace=")
-    local_nonpersistent_flags+=("--openshift-namespace=")
     flags+=("--as=")
     flags+=("--as-group=")
     flags+=("--certificate-authority=")

diff --git a/contrib/completions/zsh/oc b/contrib/completions/zsh/oc
@@ -1971,8 +1971,6 @@ _oc_adm_create-bootstrap-policy-file()
     flags_with_completion+=("--filename")
     flags_completion+=("_filedir")
     local_nonpersistent_flags+=("--filename=")
-    flags+=("--openshift-namespace=")
-    local_nonpersistent_flags+=("--openshift-namespace=")
     flags+=("--as=")
     flags+=("--as-group=")
     flags+=("--certificate-authority=")

diff --git a/contrib/completions/zsh/openshift b/contrib/completions/zsh/openshift
@@ -1806,8 +1806,6 @@ _openshift_admin_create-bootstrap-policy-file()
     flags_with_completion+=("--filename")
     flags_completion+=("_filedir")
     local_nonpersistent_flags+=("--filename=")
-    flags+=("--openshift-namespace=")
-    local_nonpersistent_flags+=("--openshift-namespace=")
     flags+=("--as=")
     flags+=("--as-group=")
     flags+=("--certificate-authority=")
@@ -7231,8 +7229,6 @@ _openshift_cli_adm_create-bootstrap-policy-file()
     flags_with_completion+=("--filename")
     flags_completion+=("_filedir")
     local_nonpersistent_flags+=("--filename=")
-    flags+=("--openshift-namespace=")
-    local_nonpersistent_flags+=("--openshift-namespace=")
     flags+=("--as=")
     flags+=("--as-group=")
     flags+=("--certificate-authority=")

diff --git a/pkg/authorization/apis/authorization/rbacconversion/conversion.go b/pkg/authorization/apis/authorization/rbacconversion/conversion.go
@@ -47,6 +47,7 @@ func Convert_authorization_ClusterRole_To_rbac_ClusterRole(in *authorizationapi.
 
 func Convert_authorization_Role_To_rbac_Role(in *authorizationapi.Role, out *rbac.Role, _ conversion.Scope) error {
 	out.ObjectMeta = in.ObjectMeta
+	out.Annotations = convert_authorization_Annotations_To_rbac_Annotations(in.Annotations)
 	out.Rules = convert_api_PolicyRules_To_rbac_PolicyRules(in.Rules)
 	return nil
 }
@@ -61,6 +62,7 @@ func Convert_authorization_ClusterRoleBinding_To_rbac_ClusterRoleBinding(in *aut
 	}
 	out.RoleRef = convert_api_RoleRef_To_rbac_RoleRef(&in.RoleRef)
 	out.ObjectMeta = in.ObjectMeta
+	out.Annotations = convert_authorization_Annotations_To_rbac_Annotations(in.Annotations)
 	return nil
 }
 
@@ -74,6 +76,7 @@ func Convert_authorization_RoleBinding_To_rbac_RoleBinding(in *authorizationapi.
 	}
 	out.RoleRef = convert_api_RoleRef_To_rbac_RoleRef(&in.RoleRef)
 	out.ObjectMeta = in.ObjectMeta
+	out.Annotations = convert_authorization_Annotations_To_rbac_Annotations(in.Annotations)
 	return nil
 }
 
@@ -172,6 +175,7 @@ func Convert_rbac_ClusterRole_To_authorization_ClusterRole(in *rbac.ClusterRole,
 
 func Convert_rbac_Role_To_authorization_Role(in *rbac.Role, out *authorizationapi.Role, _ conversion.Scope) error {
 	out.ObjectMeta = in.ObjectMeta
+	out.Annotations = convert_rbac_Annotations_To_authorization_Annotations(in.Annotations)
 	out.Rules = Convert_rbac_PolicyRules_To_authorization_PolicyRules(in.Rules)
 	return nil
 }
@@ -185,6 +189,7 @@ func Convert_rbac_ClusterRoleBinding_To_authorization_ClusterRoleBinding(in *rba
 		return err
 	}
 	out.ObjectMeta = in.ObjectMeta
+	out.Annotations = convert_rbac_Annotations_To_authorization_Annotations(in.Annotations)
 	return nil
 }
 
@@ -197,6 +202,7 @@ func Convert_rbac_RoleBinding_To_authorization_RoleBinding(in *rbac.RoleBinding,
 		return err
 	}
 	out.ObjectMeta = in.ObjectMeta
+	out.Annotations = convert_rbac_Annotations_To_authorization_Annotations(in.Annotations)
 	return nil
 }
 

diff --git a/pkg/cmd/server/admin/create_bootstrappolicy_file.go b/pkg/cmd/server/admin/create_bootstrappolicy_file.go
@@ -10,7 +10,9 @@ import (
 
 	"github.com/spf13/cobra"
 
+	"k8s.io/apimachinery/pkg/util/sets"
 	kapi "k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/apis/rbac"
 	kcmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 	kprinters "k8s.io/kubernetes/pkg/printers"
 
@@ -53,6 +55,7 @@ func NewCommandCreateBootstrapPolicyFile(commandName string, fullName string, ou
 
 	flags.StringVar(&options.File, "filename", DefaultPolicyFile, "The policy template file that will be written with roles and bindings.")
 	flags.StringVar(&options.OpenShiftSharedResourcesNamespace, "openshift-namespace", "openshift", "Namespace for shared resources.")
+	flags.MarkDeprecated("openshift-namespace", "this field is no longer supported and using it can lead to undefined behavior")
 
 	// autocompletion hints
 	cmd.MarkFlagFilename("filename")
@@ -80,11 +83,11 @@ func (o CreateBootstrapPolicyFileOptions) CreateBootstrapPolicyFile() error {
 	}
 
 	policyTemplate := &templateapi.Template{}
+	policy := bootstrappolicy.Policy()
 
-	clusterRoles := bootstrappolicy.GetBootstrapClusterRoles()
-	for i := range clusterRoles {
+	for i := range policy.ClusterRoles {
 		originObject := &authorizationapi.ClusterRole{}
-		if err := kapi.Scheme.Convert(&clusterRoles[i], originObject, nil); err != nil {
+		if err := kapi.Scheme.Convert(&policy.ClusterRoles[i], originObject, nil); err != nil {
 			return err
 		}
 		versionedObject, err := kapi.Scheme.ConvertToVersion(originObject, latest.Version)
@@ -94,10 +97,9 @@ func (o CreateBootstrapPolicyFileOptions) CreateBootstrapPolicyFile() error {
 		policyTemplate.Objects = append(policyTemplate.Objects, versionedObject)
 	}
 
-	clusterRoleBindings := bootstrappolicy.GetBootstrapClusterRoleBindings()
-	for i := range clusterRoleBindings {
+	for i := range policy.ClusterRoleBindings {
 		originObject := &authorizationapi.ClusterRoleBinding{}
-		if err := kapi.Scheme.Convert(&clusterRoleBindings[i], originObject, nil); err != nil {
+		if err := kapi.Scheme.Convert(&policy.ClusterRoleBindings[i], originObject, nil); err != nil {
 			return err
 		}
 		versionedObject, err := kapi.Scheme.ConvertToVersion(originObject, latest.Version)
@@ -107,30 +109,64 @@ func (o CreateBootstrapPolicyFileOptions) CreateBootstrapPolicyFile() error {
 		policyTemplate.Objects = append(policyTemplate.Objects, versionedObject)
 	}
 
-	openshiftRoles := bootstrappolicy.GetBootstrapOpenshiftRoles(o.OpenShiftSharedResourcesNamespace)
-	for i := range openshiftRoles {
-		originObject := &authorizationapi.Role{}
-		if err := kapi.Scheme.Convert(&openshiftRoles[i], originObject, nil); err != nil {
-			return err
+	openshiftRoles := map[string][]rbac.Role{}
+	for namespace, roles := range policy.Roles {
+		if namespace == bootstrappolicy.DefaultOpenShiftSharedResourcesNamespace {
+			r := make([]rbac.Role, len(roles))
+			for i := range roles {
+				r[i] = roles[i]
+				r[i].Namespace = o.OpenShiftSharedResourcesNamespace
+			}
+			openshiftRoles[o.OpenShiftSharedResourcesNamespace] = r
+		} else {
+			openshiftRoles[namespace] = roles
 		}
-		versionedObject, err := kapi.Scheme.ConvertToVersion(originObject, latest.Version)
-		if err != nil {
-			return err
+	}
+
+	// iterate in a defined order
+	for _, namespace := range sets.StringKeySet(openshiftRoles).List() {
+		roles := openshiftRoles[namespace]
+		for i := range roles {
+			originObject := &authorizationapi.Role{}
+			if err := kapi.Scheme.Convert(&roles[i], originObject, nil); err != nil {
+				return err
+			}
+			versionedObject, err := kapi.Scheme.ConvertToVersion(originObject, latest.Version)
+			if err != nil {
+				return err
+			}
+			policyTemplate.Objects = append(policyTemplate.Objects, versionedObject)
 		}
-		policyTemplate.Objects = append(policyTemplate.Objects, versionedObject)
 	}
 
-	openshiftRoleBindings := bootstrappolicy.GetBootstrapOpenshiftRoleBindings(o.OpenShiftSharedResourcesNamespace)
-	for i := range openshiftRoleBindings {
-		originObject := &authorizationapi.RoleBinding{}
-		if err := kapi.Scheme.Convert(&openshiftRoleBindings[i], originObject, nil); err != nil {
-			return err
+	openshiftRoleBindings := map[string][]rbac.RoleBinding{}
+	for namespace, roleBindings := range policy.RoleBindings {
+		if namespace == bootstrappolicy.DefaultOpenShiftSharedResourcesNamespace {
+			rb := make([]rbac.RoleBinding, len(roleBindings))
+			for i := range roleBindings {
+				rb[i] = roleBindings[i]
+				rb[i].Namespace = o.OpenShiftSharedResourcesNamespace
+			}
+			openshiftRoleBindings[o.OpenShiftSharedResourcesNamespace] = rb
+		} else {
+			openshiftRoleBindings[namespace] = roleBindings
 		}
-		versionedObject, err := kapi.Scheme.ConvertToVersion(originObject, latest.Version)
-		if err != nil {
-			return err
+	}
+
+	// iterate in a defined order
+	for _, namespace := range sets.StringKeySet(openshiftRoleBindings).List() {
+		roleBindings := openshiftRoleBindings[namespace]
+		for i := range roleBindings {
+			originObject := &authorizationapi.RoleBinding{}
+			if err := kapi.Scheme.Convert(&roleBindings[i], originObject, nil); err != nil {
+				return err
+			}
+			versionedObject, err := kapi.Scheme.ConvertToVersion(originObject, latest.Version)
+			if err != nil {
+				return err
+			}
+			policyTemplate.Objects = append(policyTemplate.Objects, versionedObject)
 		}
-		policyTemplate.Objects = append(policyTemplate.Objects, versionedObject)
 	}
 
 	versionedPolicyTemplate, err := kapi.Scheme.ConvertToVersion(policyTemplate, latest.Version)

diff --git a/pkg/cmd/server/bootstrappolicy/all.go b/pkg/cmd/server/bootstrappolicy/all.go
@@ -1,19 +1,14 @@
 package bootstrappolicy
 
 import (
-	"k8s.io/kubernetes/pkg/apis/rbac"
 	rbacrest "k8s.io/kubernetes/pkg/registry/rbac/rest"
 )
 
 func Policy() *rbacrest.PolicyData {
 	return &rbacrest.PolicyData{
 		ClusterRoles:        GetBootstrapClusterRoles(),
 		ClusterRoleBindings: GetBootstrapClusterRoleBindings(),
-		Roles: map[string][]rbac.Role{
-			DefaultOpenShiftSharedResourcesNamespace: GetBootstrapOpenshiftRoles(DefaultOpenShiftSharedResourcesNamespace),
-		},
-		RoleBindings: map[string][]rbac.RoleBinding{
-			DefaultOpenShiftSharedResourcesNamespace: GetBootstrapOpenshiftRoleBindings(DefaultOpenShiftSharedResourcesNamespace),
-		},
+		Roles:               GetBootstrapNamespaceRoles(),
+		RoleBindings:        GetBootstrapNamespaceRoleBindings(),
 	}
 }
diff --git a/pkg/cmd/server/bootstrappolicy/controller_policy.go b/pkg/cmd/server/bootstrappolicy/controller_policy.go
@@ -77,10 +77,12 @@ func addControllerRoleToSA(saNamespace, saName string, role rbac.ClusterRole) {
 		}
 	}
 
+	addDefaultMetadata(&role)
 	controllerRoles = append(controllerRoles, role)
 
-	controllerRoleBindings = append(controllerRoleBindings,
-		rbac.NewClusterBinding(role.Name).SAs(saNamespace, saName).BindingOrDie())
+	roleBinding := rbac.NewClusterBinding(role.Name).SAs(saNamespace, saName).BindingOrDie()
+	addDefaultMetadata(&roleBinding)
+	controllerRoleBindings = append(controllerRoleBindings, roleBinding)
 }
 
 func eventsRule() rbac.PolicyRule {
@@ -167,8 +169,9 @@ func init() {
 	})
 
 	// template-instance-controller
-	controllerRoleBindings = append(controllerRoleBindings,
-		rbac.NewClusterBinding(AdminRoleName).SAs(DefaultOpenShiftInfraNamespace, InfraTemplateInstanceControllerServiceAccountName).BindingOrDie())
+	templateInstanceController := rbac.NewClusterBinding(AdminRoleName).SAs(DefaultOpenShiftInfraNamespace, InfraTemplateInstanceControllerServiceAccountName).BindingOrDie()
+	addDefaultMetadata(&templateInstanceController)
+	controllerRoleBindings = append(controllerRoleBindings, templateInstanceController)
 
 	// origin-namespace-controller
 	addControllerRole(rbac.ClusterRole{

diff --git a/pkg/cmd/server/bootstrappolicy/dead.go b/pkg/cmd/server/bootstrappolicy/dead.go
@@ -20,11 +20,11 @@ func addDeadClusterRole(name string) {
 		}
 	}
 
-	deadClusterRoles = append(deadClusterRoles,
-		rbac.ClusterRole{
-			ObjectMeta: metav1.ObjectMeta{Name: name},
-		},
-	)
+	deadClusterRole := rbac.ClusterRole{
+		ObjectMeta: metav1.ObjectMeta{Name: name},
+	}
+	addDefaultMetadata(&deadClusterRole)
+	deadClusterRoles = append(deadClusterRoles, deadClusterRole)
 }
 
 func addDeadClusterRoleBinding(name, roleName string) {
@@ -34,12 +34,12 @@ func addDeadClusterRoleBinding(name, roleName string) {
 		}
 	}
 
-	deadClusterRoleBindings = append(deadClusterRoleBindings,
-		rbac.ClusterRoleBinding{
-			ObjectMeta: metav1.ObjectMeta{Name: name},
-			RoleRef:    rbac.RoleRef{APIGroup: rbac.GroupName, Kind: "ClusterRole", Name: roleName},
-		},
-	)
+	deadClusterRoleBinding := rbac.ClusterRoleBinding{
+		ObjectMeta: metav1.ObjectMeta{Name: name},
+		RoleRef:    rbac.RoleRef{APIGroup: rbac.GroupName, Kind: "ClusterRole", Name: roleName},
+	}
+	addDefaultMetadata(&deadClusterRoleBinding)
+	deadClusterRoleBindings = append(deadClusterRoleBindings, deadClusterRoleBinding)
 }
 
 // GetDeadClusterRoles returns cluster roles which should no longer have any permissions.