@@ -362,14 +362,14 @@ Add users or serviceaccount to a security context constraint

 == oadm policy reconcile-cluster-role-bindings

-Replace cluster role bindings to match the recommended bootstrap policy

+Update cluster role bindings to match the recommended bootstrap policy

 ====

 [options="nowrap"]

 ----

-  # Display the cluster role bindings that would be modified

-  $ oadm policy reconcile-cluster-role-bindings

+  # Display the names of cluster role bindings that would be modified

+  $ oadm policy reconcile-cluster-role-bindings -o name

   # Display the cluster role bindings that would be modified, removing any extra subjects

   $ oadm policy reconcile-cluster-role-bindings --additive-only=false

@@ -387,18 +387,22 @@ Replace cluster role bindings to match the recommended bootstrap policy

 == oadm policy reconcile-cluster-roles

-Replace cluster roles to match the recommended bootstrap policy

+Update cluster roles to match the recommended bootstrap policy

 ====

 [options="nowrap"]

 ----

-  # Display the cluster roles that would be modified

-  $ oadm policy reconcile-cluster-roles

+  # Display the names of cluster roles that would be modified

+  $ oadm policy reconcile-cluster-roles -o name

-  # Replace cluster roles that don't match the current defaults

+  # Add missing permissions to cluster roles that don't match the current defaults

   $ oadm policy reconcile-cluster-roles --confirm

+  # Add missing permissions and remove extra permissions from 

+  # cluster roles that don't match the current defaults

+  $ oadm policy reconcile-cluster-roles --additive-only=false --confirm

+

   # Display the union of the default and modified cluster roles

   $ oadm policy reconcile-cluster-roles --additive-only

 ----

@@ -362,14 +362,14 @@ Add users or serviceaccount to a security context constraint

 == oc adm policy reconcile-cluster-role-bindings

-Replace cluster role bindings to match the recommended bootstrap policy

+Update cluster role bindings to match the recommended bootstrap policy

 ====

 [options="nowrap"]

 ----

-  # Display the cluster role bindings that would be modified

-  $ oc adm policy reconcile-cluster-role-bindings

+  # Display the names of cluster role bindings that would be modified

+  $ oc adm policy reconcile-cluster-role-bindings -o name

   # Display the cluster role bindings that would be modified, removing any extra subjects

   $ oc adm policy reconcile-cluster-role-bindings --additive-only=false

@@ -387,18 +387,22 @@ Replace cluster role bindings to match the recommended bootstrap policy

 == oc adm policy reconcile-cluster-roles

-Replace cluster roles to match the recommended bootstrap policy

+Update cluster roles to match the recommended bootstrap policy

 ====

 [options="nowrap"]

 ----

-  # Display the cluster roles that would be modified

-  $ oc adm policy reconcile-cluster-roles

+  # Display the names of cluster roles that would be modified

+  $ oc adm policy reconcile-cluster-roles -o name

-  # Replace cluster roles that don't match the current defaults

+  # Add missing permissions to cluster roles that don't match the current defaults

   $ oc adm policy reconcile-cluster-roles --confirm

+  # Add missing permissions and remove extra permissions from 

+  # cluster roles that don't match the current defaults

+  $ oc adm policy reconcile-cluster-roles --additive-only=false --confirm

+

   # Display the union of the default and modified cluster roles

   $ oc adm policy reconcile-cluster-roles --additive-only

 ----

@@ -48,7 +48,7 @@ func NewCommandAdmin(name, fullName string, out io.Writer, errout io.Writer) *co

 			Message: "Basic Commands:",

 			Commands: []*cobra.Command{

 				project.NewCmdNewProject(project.NewProjectRecommendedName, fullName+" "+project.NewProjectRecommendedName, f, out),

-				policy.NewCmdPolicy(policy.PolicyRecommendedName, fullName+" "+policy.PolicyRecommendedName, f, out),

+				policy.NewCmdPolicy(policy.PolicyRecommendedName, fullName+" "+policy.PolicyRecommendedName, f, out, errout),

 				groups.NewCmdGroups(groups.GroupsRecommendedName, fullName+" "+groups.GroupsRecommendedName, f, out),

 			},

 		},

@@ -33,7 +33,7 @@ and 'scc'.

 `

 // NewCmdPolicy implements the OpenShift cli policy command

-func NewCmdPolicy(name, fullName string, f *clientcmd.Factory, out io.Writer) *cobra.Command {

+func NewCmdPolicy(name, fullName string, f *clientcmd.Factory, out, errout io.Writer) *cobra.Command {

 	// Parent command to which all subcommands are added.

 	cmds := &cobra.Command{

 		Use:   name,

@@ -86,7 +86,7 @@ func NewCmdPolicy(name, fullName string, f *clientcmd.Factory, out io.Writer) *c

 		{

 			Message: "Upgrade and repair system policy:",

 			Commands: []*cobra.Command{

-				NewCmdReconcileClusterRoles(ReconcileClusterRolesRecommendedName, fullName+" "+ReconcileClusterRolesRecommendedName, f, out),

+				NewCmdReconcileClusterRoles(ReconcileClusterRolesRecommendedName, fullName+" "+ReconcileClusterRolesRecommendedName, f, out, errout),

 				NewCmdReconcileClusterRoleBindings(ReconcileClusterRoleBindingsRecommendedName, fullName+" "+ReconcileClusterRoleBindingsRecommendedName, f, out),

 				NewCmdReconcileSCC(ReconcileSCCRecommendedName, fullName+" "+ReconcileSCCRecommendedName, f, out),

 			},

@@ -41,7 +41,7 @@ type ReconcileClusterRoleBindingsOptions struct {

 const (

 	reconcileBindingsLong = `

-Replace cluster role bindings to match the recommended bootstrap policy

+Update cluster role bindings to match the recommended bootstrap policy

 This command will inspect the cluster role bindings against the recommended bootstrap policy.

 Any cluster role binding that does not match will be replaced by the recommended bootstrap role binding.

@@ -49,8 +49,8 @@ This command will not remove any additional cluster role bindings.

 You can see which recommended cluster role bindings have changed by choosing an output type.`

-	reconcileBindingsExample = `  # Display the cluster role bindings that would be modified

-  $ %[1]s

+	reconcileBindingsExample = `  # Display the names of cluster role bindings that would be modified

+  $ %[1]s -o name

   # Display the cluster role bindings that would be modified, removing any extra subjects

   $ %[1]s --additive-only=false

@@ -77,7 +77,7 @@ func NewCmdReconcileClusterRoleBindings(name, fullName string, f *clientcmd.Fact

 	cmd := &cobra.Command{

 		Use:     name + " [ClusterRoleName]...",

-		Short:   "Replace cluster role bindings to match the recommended bootstrap policy",

+		Short:   "Update cluster role bindings to match the recommended bootstrap policy",

 		Long:    reconcileBindingsLong,

 		Example: fmt.Sprintf(reconcileBindingsExample, fullName),

 		Run: func(cmd *cobra.Command, args []string) {

@@ -140,9 +140,6 @@ func (o *ReconcileClusterRoleBindingsOptions) Validate() error {

 	if o.RoleBindingClient == nil {

 		return errors.New("a role binding client is required")

 	}

-	if o.Output != "yaml" && o.Output != "json" && o.Output != "" {

-		return fmt.Errorf("unknown output specified: %s", o.Output)

-	}

 	return nil

 }

@@ -21,6 +21,9 @@ import (

 	"github.com/openshift/origin/pkg/cmd/util/clientcmd"

 )

+// ReconcileProtectAnnotation is the name of an annotation which prevents reconciliation if set to "true"

+const ReconcileProtectAnnotation = "openshift.io/reconcile-protect"

+

 // ReconcileClusterRolesRecommendedName is the recommended command name

 const ReconcileClusterRolesRecommendedName = "reconcile-cluster-roles"

@@ -33,6 +36,7 @@ type ReconcileClusterRolesOptions struct {

 	Union     bool

 	Out    io.Writer

+	ErrOut io.Writer

 	Output string

 	RoleClient client.ClusterRoleInterface

@@ -40,35 +44,42 @@ type ReconcileClusterRolesOptions struct {

 const (

 	reconcileLong = `

-Replace cluster roles to match the recommended bootstrap policy

+Update cluster roles to match the recommended bootstrap policy

-This command will inspect the cluster roles against the recommended bootstrap policy.  Any cluster role

+This command will compare cluster roles against the recommended bootstrap policy.  Any cluster role

 that does not match will be replaced by the recommended bootstrap role.  This command will not remove

 any additional cluster role.

-You can see which cluster role have recommended changed by choosing an output type.`

+Cluster roles with the annotation %s set to "true" are skipped.

+

+You can see which cluster roles have recommended changed by choosing an output type.`

-	reconcileExample = `  # Display the cluster roles that would be modified

-  $ %[1]s

+	reconcileExample = `  # Display the names of cluster roles that would be modified

+  $ %[1]s -o name

-  # Replace cluster roles that don't match the current defaults

+  # Add missing permissions to cluster roles that don't match the current defaults

   $ %[1]s --confirm

+  # Add missing permissions and remove extra permissions from 

+  # cluster roles that don't match the current defaults

+  $ %[1]s --additive-only=false --confirm

+

   # Display the union of the default and modified cluster roles

   $ %[1]s --additive-only`

 )

 // NewCmdReconcileClusterRoles implements the OpenShift cli reconcile-cluster-roles command

-func NewCmdReconcileClusterRoles(name, fullName string, f *clientcmd.Factory, out io.Writer) *cobra.Command {

+func NewCmdReconcileClusterRoles(name, fullName string, f *clientcmd.Factory, out, errout io.Writer) *cobra.Command {

 	o := &ReconcileClusterRolesOptions{

-		Out:   out,

-		Union: true,

+		Out:    out,

+		ErrOut: errout,

+		Union:  true,

 	}

 	cmd := &cobra.Command{

 		Use:     name + " [ClusterRoleName]...",

-		Short:   "Replace cluster roles to match the recommended bootstrap policy",

-		Long:    reconcileLong,

+		Short:   "Update cluster roles to match the recommended bootstrap policy",

+		Long:    fmt.Sprintf(reconcileLong, ReconcileProtectAnnotation),

 		Example: fmt.Sprintf(reconcileExample, fullName),

 		Run: func(cmd *cobra.Command, args []string) {

 			if err := o.Complete(cmd, f, args); err != nil {

@@ -126,19 +137,23 @@ func (o *ReconcileClusterRolesOptions) Validate() error {

 	if o.RoleClient == nil {

 		return errors.New("a role client is required")

 	}

-	if o.Output != "yaml" && o.Output != "json" && o.Output != "" {

-		return fmt.Errorf("unknown output specified: %s", o.Output)

-	}

 	return nil

 }

 // RunReconcileClusterRoles contains all the necessary functionality for the OpenShift cli reconcile-cluster-roles command

 func (o *ReconcileClusterRolesOptions) RunReconcileClusterRoles(cmd *cobra.Command, f *clientcmd.Factory) error {

-	changedClusterRoles, err := o.ChangedClusterRoles()

+	changedClusterRoles, skippedClusterRoles, err := o.ChangedClusterRoles()

 	if err != nil {

 		return err

 	}

+	if len(skippedClusterRoles) > 0 {

+		fmt.Fprintf(o.ErrOut, "Skipped reconciling roles with the annotation %s=true\n", ReconcileProtectAnnotation)

+		for _, role := range skippedClusterRoles {

+			fmt.Fprintf(o.ErrOut, "skipped: clusterrole/%s\n", role.Name)

+		}

+	}

+

 	if len(changedClusterRoles) == 0 {

 		return nil

 	}

@@ -164,8 +179,9 @@ func (o *ReconcileClusterRolesOptions) RunReconcileClusterRoles(cmd *cobra.Comma

 // ChangedClusterRoles returns the roles that must be created and/or updated to

 // match the recommended bootstrap policy

-func (o *ReconcileClusterRolesOptions) ChangedClusterRoles() ([]*authorizationapi.ClusterRole, error) {

+func (o *ReconcileClusterRolesOptions) ChangedClusterRoles() ([]*authorizationapi.ClusterRole, []*authorizationapi.ClusterRole, error) {

 	changedRoles := []*authorizationapi.ClusterRole{}

+	skippedRoles := []*authorizationapi.ClusterRole{}

 	rolesToReconcile := sets.NewString(o.RolesToReconcile...)

 	rolesNotFound := sets.NewString(o.RolesToReconcile...)

@@ -183,7 +199,7 @@ func (o *ReconcileClusterRolesOptions) ChangedClusterRoles() ([]*authorizationap

 			continue

 		}

 		if err != nil {

-			return nil, err

+			return nil, nil, err

 		}

 		// Copy any existing labels/annotations, so the displayed update is correct

@@ -202,16 +218,21 @@ func (o *ReconcileClusterRolesOptions) ChangedClusterRoles() ([]*authorizationap

 			if o.Union {

 				expectedClusterRole.Rules = append(expectedClusterRole.Rules, extraRules...)

 			}

-			changedRoles = append(changedRoles, expectedClusterRole)

+

+			if actualClusterRole.Annotations[ReconcileProtectAnnotation] == "true" {

+				skippedRoles = append(skippedRoles, expectedClusterRole)

+			} else {

+				changedRoles = append(changedRoles, expectedClusterRole)

+			}

 		}

 	}

 	if len(rolesNotFound) != 0 {

 		// return the known changes and the error so that a caller can decide if he wants a partial update

-		return changedRoles, fmt.Errorf("did not find requested cluster role %s", rolesNotFound.List())

+		return changedRoles, skippedRoles, fmt.Errorf("did not find requested cluster role %s", rolesNotFound.List())

 	}

-	return changedRoles, nil

+	return changedRoles, skippedRoles, nil

 }

 // ReplaceChangedRoles will reconcile all the changed roles back to the recommended bootstrap policy

@@ -124,9 +124,6 @@ func (o *ReconcileSCCOptions) Validate() error {

 	if o.SCCClient == nil {

 		return errors.New("a SCC client is required")

 	}

-	if o.Output != "yaml" && o.Output != "json" && o.Output != "" {

-		return fmt.Errorf("unknown output specified: %s", o.Output)

-	}

 	if _, err := o.NSClient.Get(o.InfraNamespace); err != nil {

 		return fmt.Errorf("%s is not a valid namespace", o.InfraNamespace)

 	}

@@ -56,7 +56,7 @@ func (d *ClusterRoles) Check() types.DiagnosticResult {

 		RoleClient: d.ClusterRolesClient.ClusterRoles(),

 	}

-	changedClusterRoles, err := reconcileOptions.ChangedClusterRoles()

+	changedClusterRoles, _, err := reconcileOptions.ChangedClusterRoles()

 	if err != nil {

 		r.Error("CRD1000", err, fmt.Sprintf("Error inspecting ClusterRoles: %v", err))

 		return r

@@ -24,8 +24,8 @@ trap os::test::junit::reconcile_output EXIT

   oc delete users/orphaned-user

   oc delete identities/anypassword:orphaned-user

   oc delete identities/anypassword:cascaded-user

-  oadm policy reconcile-cluster-roles --confirm

-  oadm policy reconcile-cluster-role-bindings --confirm

+  oadm policy reconcile-cluster-roles --confirm --additive-only=false

+  oadm policy reconcile-cluster-role-bindings --confirm --additive-only=false

 ) &>/dev/null

@@ -176,11 +176,28 @@ os::cmd::expect_failure 'oc get clusterrole/cluster-status'

 os::cmd::expect_success 'oadm policy reconcile-cluster-roles clusterrole/cluster-status --confirm'

 os::cmd::expect_success 'oc get clusterrole/cluster-status'

-os::cmd::expect_success 'oc replace --force -f ./test/fixtures/basic-user.json'

+# test reconciliation protection by replacing the basic-user role with one that has missing default permissions, and extra non-default permissions

+os::cmd::expect_success 'oc replace --force -f ./test/fixtures/basic-user-with-groups-without-projectrequests.yaml'

+# 1. mark the role as protected, and ensure the role is skipped by reconciliation

+os::cmd::expect_success 'oc annotate clusterrole/basic-user openshift.io/reconcile-protect=true'

+os::cmd::expect_success_and_text     'oadm policy reconcile-cluster-roles basic-user --additive-only=false --confirm' 'skipped: clusterrole/basic-user'

+# 2. unmark the role as protected, and ensure reconcile expects to remove extra permissions, and put back removed permissions

+os::cmd::expect_success 'oc annotate clusterrole/basic-user openshift.io/reconcile-protect=false --overwrite'

+os::cmd::expect_success_and_text     'oc get clusterrole/basic-user -o jsonpath="{.rules[*].resources}"' 'groups'

+os::cmd::expect_success_and_not_text 'oc get clusterrole/basic-user -o jsonpath="{.rules[*].resources}"' 'projectrequests'

+os::cmd::expect_success_and_not_text 'oadm policy reconcile-cluster-roles basic-user -o jsonpath="{.items[*].rules[*].resources}" --additive-only=false' 'groups'

+os::cmd::expect_success_and_text     'oadm policy reconcile-cluster-roles basic-user -o jsonpath="{.items[*].rules[*].resources}" --additive-only=false' 'projectrequests'

+# reconcile updates the role

+os::cmd::expect_success_and_text     'oadm policy reconcile-cluster-roles basic-user --additive-only=false --confirm' 'clusterrole/basic-user'

+# a second reconcile doesn't need to update the role

+os::cmd::expect_success_and_not_text 'oadm policy reconcile-cluster-roles basic-user --additive-only=false --confirm' 'clusterrole/basic-user'

+

+# test label/annotation reconciliation by replacing the basic-user role with one that has custom labels, annotations, and permissions

+os::cmd::expect_success 'oc replace --force -f ./test/fixtures/basic-user-with-annotations-labels-groups-without-projectrequests.yaml'

 # display shows customized labels/annotations

 os::cmd::expect_success_and_text 'oadm policy reconcile-cluster-roles' 'custom-label'

 os::cmd::expect_success_and_text 'oadm policy reconcile-cluster-roles' 'custom-annotation'

-os::cmd::expect_success 'oadm policy reconcile-cluster-roles --additive-only --confirm'

+os::cmd::expect_success_and_text 'oadm policy reconcile-cluster-roles --additive-only --confirm' 'clusterrole/basic-user'

 # reconcile preserves added rules, labels, and annotations

 os::cmd::expect_success_and_text 'oc get clusterroles/basic-user -o json' 'custom-label'

 os::cmd::expect_success_and_text 'oc get clusterroles/basic-user -o json' 'custom-annotation'

@@ -46,7 +46,7 @@ os::cmd::expect_failure_and_text 'oc process template-name key=value other=foo -

 required_params="${OS_ROOT}/test/fixtures/template_required_params.yaml"

 # providing something other than a template is not OK

-os::cmd::expect_failure_and_text "oc process -f '${OS_ROOT}/test/fixtures/basic-user.json'" 'not a valid Template but'

+os::cmd::expect_failure_and_text "oc process -f '${OS_ROOT}/test/fixtures/basic-users-binding.json'" 'not a valid Template but'

 # not providing required parameter should fail

 os::cmd::expect_failure_and_text "oc process -f '${required_params}'" 'parameter required_param is required and must be specified'

new file mode 100644

@@ -0,0 +1,57 @@

+apiVersion: v1

+kind: ClusterRole

+metadata:

+  name: basic-user

+  labels:

+    # custom labels

+    custom-label: "value"

+  annotations:

+    # custom annotations

+    custom-annotation: "value"

+rules:

+- apiGroups: null

+  attributeRestrictions: null

+  resourceNames:

+  - "~"

+  resources:

+  - users

+  # add an extra resource permission:

+  - groups

+  verbs:

+  - get

+# remove default permission:

+# - apiGroups: null

+#   attributeRestrictions: null

+#   resources:

+#   - projectrequests

+#   verbs:

+#   - list

+- apiGroups: null

+  attributeRestrictions: null

+  resources:

+  - clusterroles

+  verbs:

+  - get

+  - list

+- apiGroups: null

+  attributeRestrictions: null

+  resources:

+  - projects

+  verbs:

+  - list

+  - watch

+- apiGroups: null

+  attributeRestrictions:

+    apiVersion: v1

+    kind: IsPersonalSubjectAccessReview

+  resources:

+  - localsubjectaccessreviews

+  - subjectaccessreviews

+  verbs:

+  - create

+- apiGroups: null

+  attributeRestrictions: null

+  resources:

+  - selfsubjectrulesreviews

+  verbs:

+  - create

new file mode 100644

@@ -0,0 +1,51 @@

+apiVersion: v1

+kind: ClusterRole

+metadata:

+  name: basic-user

+rules:

+- apiGroups: null

+  attributeRestrictions: null

+  resourceNames:

+  - "~"

+  resources:

+  - users

+  # add an extra resource permission:

+  - groups

+  verbs:

+  - get

+# remove a default permission:

+# - apiGroups: null

+#   attributeRestrictions: null

+#   resources:

+#   - projectrequests

+#   verbs:

+#   - list

+- apiGroups: null

+  attributeRestrictions: null

+  resources:

+  - clusterroles

+  verbs:

+  - get

+  - list

+- apiGroups: null

+  attributeRestrictions: null

+  resources:

+  - projects

+  verbs:

+  - list

+  - watch

+- apiGroups: null

+  attributeRestrictions:

+    apiVersion: v1

+    kind: IsPersonalSubjectAccessReview

+  resources:

+  - localsubjectaccessreviews

+  - subjectaccessreviews

+  verbs:

+  - create

+- apiGroups: null

+  attributeRestrictions: null

+  resources:

+  - selfsubjectrulesreviews

+  verbs:

+  - create

deleted file mode 100644

@@ -1,83 +0,0 @@

-{

-    "kind": "ClusterRole",

-    "apiVersion": "v1",

-    "metadata": {

-        "name": "basic-user",

-        "selfLink": "/osapi/v1beta3/clusterroles/basic-user",

-        "uid": "86a5ce7c-3f44-11e5-a9a6-080027c5bfa9",

-        "resourceVersion": "18",

-        "creationTimestamp": "2015-08-10T09:45:25Z",

-        "labels": {

-        	"custom-label":"value"

-        },

-        "annotations": {

-        	"custom-annotation":"value"

-        }

-    },

-    "rules": [

-        {

-            "verbs": [

-                "get"

-            ],

-            "attributeRestrictions": null,

-            "resources": [

-                "users"

-            ],

-            "resourceNames": [

-                "~"

-            ]

-        },

-        {

-            "verbs": [

-                "get"

-            ],

-            "attributeRestrictions": null,

-            "resources": [

-                "groups"

-            ],

-            "resourceNames": [

-                "~"

-            ]

-        },

-        {

-            "verbs": [

-                "list"

-            ],

-            "attributeRestrictions": null,

-            "resources": [

-                "projectrequests"

-            ]

-        },

-        {

-            "verbs": [

-                "get",

-                "list"

-            ],

-            "attributeRestrictions": null,

-            "resources": [

-                "clusterroles"

-            ]

-        },

-        {

-            "verbs": [

-                "list"

-            ],

-            "attributeRestrictions": null,

-            "resources": [

-                "projects"

-            ]

-        },

-        {

-            "verbs": [

-                "create"

-            ],

-            "attributeRestrictions": {

-                "kind": "IsPersonalSubjectAccessReview",

-                "apiVersion": "v1"

-            },

-            "resources": [

-                "subjectaccessreviews"

-            ]

-        }

-    ]

-}