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

Bootstrap Kube namespaced roles and bindings #16517

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 0 additions & 2 deletions contrib/completions/bash/oadm
Original file line number Diff line number Diff line change
Expand Up @@ -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=")
Expand Down
2 changes: 0 additions & 2 deletions contrib/completions/bash/oc
Original file line number Diff line number Diff line change
Expand Up @@ -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=")
Expand Down
4 changes: 0 additions & 4 deletions contrib/completions/bash/openshift
Original file line number Diff line number Diff line change
Expand Up @@ -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=")
Expand Down Expand Up @@ -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=")
Expand Down
2 changes: 0 additions & 2 deletions contrib/completions/zsh/oadm
Original file line number Diff line number Diff line change
Expand Up @@ -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=")
Expand Down
2 changes: 0 additions & 2 deletions contrib/completions/zsh/oc
Original file line number Diff line number Diff line change
Expand Up @@ -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=")
Expand Down
4 changes: 0 additions & 4 deletions contrib/completions/zsh/openshift
Original file line number Diff line number Diff line change
Expand Up @@ -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=")
Expand Down Expand Up @@ -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=")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}

Expand All @@ -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
}

Expand Down Expand Up @@ -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
}
Expand All @@ -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
}

Expand All @@ -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
}

Expand Down
84 changes: 60 additions & 24 deletions pkg/cmd/server/admin/create_bootstrappolicy_file.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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")
Copy link
Contributor

Choose a reason for hiding this comment

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

this entire command should just die.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

3.8

Copy link
Contributor

Choose a reason for hiding this comment

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

We got rid of openshift-infra choice. I don't remember removing the shared namespace choice. If openshift-infra was changed, we simply created what needed to be. If they used a custom shared namespace, we cannot recreate that for them.

@smarterclayton its easier if we remove it, but I'm not sure about breaks. Opinion?

Copy link
Contributor

Choose a reason for hiding this comment

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

We got rid of openshift-infra choice. I don't remember removing the shared namespace choice. If openshift-infra was changed, we simply created what needed to be. If they used a custom shared namespace, we cannot recreate that for them.

@smarterclayton its easier if we remove it, but I'm not sure about breaks. Opinion?

Ansible doesn't allow it to be set. If a user changes it, I think its up to him to make sure all his roles are ready

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Deprecate in 3.7 and remove in 3.8 seems reasonable to me. Removing it completely in 3.7 seems a bit sudden, but I have a feeling that no sane person ever messed with this so I can stomp on it if you really want.

Copy link
Contributor

Choose a reason for hiding this comment

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

Deprecate in 3.7 and remove in 3.8 seems reasonable to me. Removing it completely in 3.7 seems a bit sudden, but I have a feeling that no sane person ever messed with this so I can stomp on it if you really want.

@enj You already have removed it in this pull. You just don't realize that you have. It no longer functions as it used to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm yeah I killed the bootstrapping of it which is what matters.


// autocompletion hints
cmd.MarkFlagFilename("filename")
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down
9 changes: 2 additions & 7 deletions pkg/cmd/server/bootstrappolicy/all.go
Original file line number Diff line number Diff line change
@@ -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(),
}
}
11 changes: 7 additions & 4 deletions pkg/cmd/server/bootstrappolicy/controller_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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{
Expand Down
22 changes: 11 additions & 11 deletions pkg/cmd/server/bootstrappolicy/dead.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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.
Expand Down
Loading