Browse code

reconcile-cluster-role-bindings command

Jordan Liggitt authored on 2015/10/03 00:02:13
Showing 10 changed files
... ...
@@ -389,6 +389,34 @@ _oadm_policy_reconcile-cluster-roles()
389 389
     must_have_one_noun=()
390 390
 }
391 391
 
392
+_oadm_policy_reconcile-cluster-role-bindings()
393
+{
394
+    last_command="oadm_policy_reconcile-cluster-role-bindings"
395
+    commands=()
396
+
397
+    flags=()
398
+    two_word_flags=()
399
+    flags_with_completion=()
400
+    flags_completion=()
401
+
402
+    flags+=("--additive-only")
403
+    flags+=("--confirm")
404
+    flags+=("--exclude-groups=")
405
+    flags+=("--exclude-users=")
406
+    flags+=("--no-headers")
407
+    flags+=("--output=")
408
+    two_word_flags+=("-o")
409
+    flags+=("--output-version=")
410
+    flags+=("--show-all")
411
+    flags+=("-a")
412
+    flags+=("--sort-by=")
413
+    flags+=("--template=")
414
+    two_word_flags+=("-t")
415
+
416
+    must_have_one_flag=()
417
+    must_have_one_noun=()
418
+}
419
+
392 420
 _oadm_policy()
393 421
 {
394 422
     last_command="oadm_policy"
... ...
@@ -405,6 +433,7 @@ _oadm_policy()
405 405
     commands+=("add-cluster-role-to-group")
406 406
     commands+=("remove-cluster-role-from-group")
407 407
     commands+=("reconcile-cluster-roles")
408
+    commands+=("reconcile-cluster-role-bindings")
408 409
 
409 410
     flags=()
410 411
     two_word_flags=()
... ...
@@ -819,6 +819,34 @@ _openshift_admin_policy_reconcile-cluster-roles()
819 819
     must_have_one_noun=()
820 820
 }
821 821
 
822
+_openshift_admin_policy_reconcile-cluster-role-bindings()
823
+{
824
+    last_command="openshift_admin_policy_reconcile-cluster-role-bindings"
825
+    commands=()
826
+
827
+    flags=()
828
+    two_word_flags=()
829
+    flags_with_completion=()
830
+    flags_completion=()
831
+
832
+    flags+=("--additive-only")
833
+    flags+=("--confirm")
834
+    flags+=("--exclude-groups=")
835
+    flags+=("--exclude-users=")
836
+    flags+=("--no-headers")
837
+    flags+=("--output=")
838
+    two_word_flags+=("-o")
839
+    flags+=("--output-version=")
840
+    flags+=("--show-all")
841
+    flags+=("-a")
842
+    flags+=("--sort-by=")
843
+    flags+=("--template=")
844
+    two_word_flags+=("-t")
845
+
846
+    must_have_one_flag=()
847
+    must_have_one_noun=()
848
+}
849
+
822 850
 _openshift_admin_policy()
823 851
 {
824 852
     last_command="openshift_admin_policy"
... ...
@@ -835,6 +863,7 @@ _openshift_admin_policy()
835 835
     commands+=("add-cluster-role-to-group")
836 836
     commands+=("remove-cluster-role-from-group")
837 837
     commands+=("reconcile-cluster-roles")
838
+    commands+=("reconcile-cluster-role-bindings")
838 839
 
839 840
     flags=()
840 841
     two_word_flags=()
... ...
@@ -201,6 +201,31 @@ Manage nodes - list pods, evacuate, or mark ready
201 201
 ====
202 202
 
203 203
 
204
+== oadm policy reconcile-cluster-role-bindings
205
+Replace cluster role bindings to match the recommended bootstrap policy
206
+
207
+====
208
+
209
+[options="nowrap"]
210
+----
211
+  # Display the cluster role bindings that would be modified
212
+  $ openshift admin policy reconcile-cluster-role-bindings
213
+
214
+  # Display the cluster role bindings that would be modified, removing any extra subjects
215
+  $ openshift admin policy reconcile-cluster-role-bindings --additive-only=false
216
+
217
+  # Update cluster role bindings that don't match the current defaults
218
+  $ openshift admin policy reconcile-cluster-role-bindings --confirm
219
+
220
+  # Update cluster role bindings that don't match the current defaults, avoid adding roles to the system:authenticated group
221
+  $ openshift admin policy reconcile-cluster-role-bindings --confirm --exclude-groups=system:authenticated
222
+
223
+  # Update cluster role bindings that don't match the current defaults, removing any extra subjects from the binding
224
+  $ openshift admin policy reconcile-cluster-role-bindings --confirm --additive-only=false
225
+----
226
+====
227
+
228
+
204 229
 == oadm policy reconcile-cluster-roles
205 230
 Replace cluster roles to match the recommended bootstrap policy
206 231
 
... ...
@@ -45,6 +45,7 @@ func NewCmdPolicy(name, fullName string, f *clientcmd.Factory, out io.Writer) *c
45 45
 	cmds.AddCommand(NewCmdAddClusterRoleToGroup(AddClusterRoleToGroupRecommendedName, fullName+" "+AddClusterRoleToGroupRecommendedName, f, out))
46 46
 	cmds.AddCommand(NewCmdRemoveClusterRoleFromGroup(RemoveClusterRoleFromGroupRecommendedName, fullName+" "+RemoveClusterRoleFromGroupRecommendedName, f, out))
47 47
 	cmds.AddCommand(NewCmdReconcileClusterRoles(ReconcileClusterRolesRecommendedName, fullName+" "+ReconcileClusterRolesRecommendedName, f, out))
48
+	cmds.AddCommand(NewCmdReconcileClusterRoleBindings(ReconcileClusterRoleBindingsRecommendedName, fullName+" "+ReconcileClusterRoleBindingsRecommendedName, f, out))
48 49
 
49 50
 	return cmds
50 51
 }
51 52
new file mode 100644
... ...
@@ -0,0 +1,307 @@
0
+package policy
1
+
2
+import (
3
+	"errors"
4
+	"fmt"
5
+	"io"
6
+
7
+	"github.com/spf13/cobra"
8
+
9
+	kapi "k8s.io/kubernetes/pkg/api"
10
+	kapierrors "k8s.io/kubernetes/pkg/api/errors"
11
+	kcmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
12
+	"k8s.io/kubernetes/pkg/util"
13
+
14
+	authorizationapi "github.com/openshift/origin/pkg/authorization/api"
15
+	"github.com/openshift/origin/pkg/client"
16
+	"github.com/openshift/origin/pkg/cmd/server/bootstrappolicy"
17
+	"github.com/openshift/origin/pkg/cmd/util/clientcmd"
18
+	uservalidation "github.com/openshift/origin/pkg/user/api/validation"
19
+)
20
+
21
+// ReconcileClusterRoleBindingsRecommendedName is the recommended command name
22
+const ReconcileClusterRoleBindingsRecommendedName = "reconcile-cluster-role-bindings"
23
+
24
+type ReconcileClusterRoleBindingsOptions struct {
25
+	Confirmed bool
26
+	Union     bool
27
+
28
+	ExcludeSubjects []kapi.ObjectReference
29
+
30
+	Out    io.Writer
31
+	Output string
32
+
33
+	RoleBindingClient client.ClusterRoleBindingInterface
34
+}
35
+
36
+const (
37
+	reconcileBindingsLong = `
38
+Replace cluster role bindings to match the recommended bootstrap policy
39
+
40
+This command will inspect the cluster role bindings against the recommended bootstrap policy.
41
+Any cluster role binding that does not match will be replaced by the recommended bootstrap role binding.
42
+This command will not remove any additional cluster role bindings.
43
+
44
+You can see which recommended cluster role bindings have changed by choosing an output type.`
45
+
46
+	reconcileBindingsExample = `  # Display the cluster role bindings that would be modified
47
+  $ %[1]s
48
+
49
+  # Display the cluster role bindings that would be modified, removing any extra subjects
50
+  $ %[1]s --additive-only=false
51
+
52
+  # Update cluster role bindings that don't match the current defaults
53
+  $ %[1]s --confirm
54
+
55
+  # Update cluster role bindings that don't match the current defaults, avoid adding roles to the system:authenticated group
56
+  $ %[1]s --confirm --exclude-groups=system:authenticated
57
+
58
+  # Update cluster role bindings that don't match the current defaults, removing any extra subjects from the binding
59
+  $ %[1]s --confirm --additive-only=false`
60
+)
61
+
62
+// NewCmdReconcileClusterRoleBindings implements the OpenShift cli reconcile-cluster-role-bindings command
63
+func NewCmdReconcileClusterRoleBindings(name, fullName string, f *clientcmd.Factory, out io.Writer) *cobra.Command {
64
+	o := &ReconcileClusterRoleBindingsOptions{
65
+		Out:   out,
66
+		Union: true,
67
+	}
68
+
69
+	excludeUsers := util.StringList{}
70
+	excludeGroups := util.StringList{}
71
+
72
+	cmd := &cobra.Command{
73
+		Use:     name,
74
+		Short:   "Replace cluster role bindings to match the recommended bootstrap policy",
75
+		Long:    reconcileBindingsLong,
76
+		Example: fmt.Sprintf(reconcileBindingsExample, fullName),
77
+		Run: func(cmd *cobra.Command, args []string) {
78
+			if err := o.Complete(cmd, f, args, excludeUsers, excludeGroups); err != nil {
79
+				kcmdutil.CheckErr(err)
80
+			}
81
+
82
+			if err := o.Validate(); err != nil {
83
+				kcmdutil.CheckErr(kcmdutil.UsageError(cmd, err.Error()))
84
+			}
85
+
86
+			if err := o.RunReconcileClusterRoleBindings(cmd, f); err != nil {
87
+				kcmdutil.CheckErr(err)
88
+			}
89
+		},
90
+	}
91
+
92
+	cmd.Flags().BoolVar(&o.Confirmed, "confirm", o.Confirmed, "Specify that cluster role bindings should be modified. Defaults to false, displaying what would be replaced but not actually replacing anything.")
93
+	cmd.Flags().BoolVar(&o.Union, "additive-only", o.Union, "Preserves extra subjects in cluster role bindings.")
94
+	cmd.Flags().Var(&excludeUsers, "exclude-users", "Do not add cluster role bindings for these user names.")
95
+	cmd.Flags().Var(&excludeGroups, "exclude-groups", "Do not add cluster role bindings for these group names.")
96
+	kcmdutil.AddPrinterFlags(cmd)
97
+	cmd.Flags().Lookup("output").DefValue = "yaml"
98
+	cmd.Flags().Lookup("output").Value.Set("yaml")
99
+
100
+	return cmd
101
+}
102
+
103
+func (o *ReconcileClusterRoleBindingsOptions) Complete(cmd *cobra.Command, f *clientcmd.Factory, args []string, excludeUsers, excludeGroups util.StringList) error {
104
+	if len(args) != 0 {
105
+		return kcmdutil.UsageError(cmd, "no arguments are allowed")
106
+	}
107
+
108
+	oclient, _, err := f.Clients()
109
+	if err != nil {
110
+		return err
111
+	}
112
+	o.RoleBindingClient = oclient.ClusterRoleBindings()
113
+
114
+	o.Output = kcmdutil.GetFlagString(cmd, "output")
115
+
116
+	o.ExcludeSubjects = authorizationapi.BuildSubjects(excludeUsers, excludeGroups, uservalidation.ValidateUserName, uservalidation.ValidateGroupName)
117
+
118
+	return nil
119
+}
120
+
121
+func (o *ReconcileClusterRoleBindingsOptions) Validate() error {
122
+	if o.RoleBindingClient == nil {
123
+		return errors.New("a role binding client is required")
124
+	}
125
+	if o.Output != "yaml" && o.Output != "json" && o.Output != "" {
126
+		return fmt.Errorf("unknown output specified: %s", o.Output)
127
+	}
128
+	return nil
129
+}
130
+
131
+// ReconcileClusterRoleBindingsOptions contains all the necessary functionality for the OpenShift cli reconcile-cluster-role-bindings command
132
+func (o *ReconcileClusterRoleBindingsOptions) RunReconcileClusterRoleBindings(cmd *cobra.Command, f *clientcmd.Factory) error {
133
+	changedClusterRoleBindings, err := o.ChangedClusterRoleBindings()
134
+	if err != nil {
135
+		return err
136
+	}
137
+
138
+	if len(changedClusterRoleBindings) == 0 {
139
+		return nil
140
+	}
141
+
142
+	if (len(o.Output) != 0) && !o.Confirmed {
143
+		list := &kapi.List{}
144
+		for _, item := range changedClusterRoleBindings {
145
+			list.Items = append(list.Items, item)
146
+		}
147
+
148
+		if err := f.Factory.PrintObject(cmd, list, o.Out); err != nil {
149
+			return err
150
+		}
151
+	}
152
+
153
+	if o.Confirmed {
154
+		return o.ReplaceChangedRoleBindings(changedClusterRoleBindings)
155
+	}
156
+
157
+	return nil
158
+}
159
+
160
+// ChangedClusterRoleBindings returns the role bindings that must be created and/or updated to
161
+// match the recommended bootstrap policy
162
+func (o *ReconcileClusterRoleBindingsOptions) ChangedClusterRoleBindings() ([]*authorizationapi.ClusterRoleBinding, error) {
163
+	changedRoleBindings := []*authorizationapi.ClusterRoleBinding{}
164
+
165
+	bootstrapClusterRoleBindings := bootstrappolicy.GetBootstrapClusterRoleBindings()
166
+	for i := range bootstrapClusterRoleBindings {
167
+		expectedClusterRoleBinding := &bootstrapClusterRoleBindings[i]
168
+
169
+		actualClusterRoleBinding, err := o.RoleBindingClient.Get(expectedClusterRoleBinding.Name)
170
+		if kapierrors.IsNotFound(err) {
171
+			// Remove excluded subjects from the new role binding
172
+			expectedClusterRoleBinding.Subjects, _ = diff(expectedClusterRoleBinding.Subjects, o.ExcludeSubjects)
173
+			changedRoleBindings = append(changedRoleBindings, expectedClusterRoleBinding)
174
+			continue
175
+		}
176
+		if err != nil {
177
+			return nil, err
178
+		}
179
+
180
+		// Copy any existing labels/annotations, so the displayed update is correct
181
+		// This assumes bootstrap role bindings will not set any labels/annotations
182
+		// These aren't actually used during update; the latest labels/annotations are pulled from the existing object again
183
+		expectedClusterRoleBinding.Labels = actualClusterRoleBinding.Labels
184
+		expectedClusterRoleBinding.Annotations = actualClusterRoleBinding.Annotations
185
+
186
+		if updatedClusterRoleBinding, needsUpdating := computeUpdatedBinding(*expectedClusterRoleBinding, *actualClusterRoleBinding, o.ExcludeSubjects, o.Union); needsUpdating {
187
+			changedRoleBindings = append(changedRoleBindings, updatedClusterRoleBinding)
188
+		}
189
+	}
190
+
191
+	return changedRoleBindings, nil
192
+}
193
+
194
+// ReplaceChangedRoleBindings will reconcile all the changed system role bindings back to the recommended bootstrap policy
195
+func (o *ReconcileClusterRoleBindingsOptions) ReplaceChangedRoleBindings(changedRoleBindings []*authorizationapi.ClusterRoleBinding) error {
196
+	for i := range changedRoleBindings {
197
+		roleBinding, err := o.RoleBindingClient.Get(changedRoleBindings[i].Name)
198
+		if err != nil && !kapierrors.IsNotFound(err) {
199
+			return err
200
+		}
201
+
202
+		if kapierrors.IsNotFound(err) {
203
+			createdRoleBinding, err := o.RoleBindingClient.Create(changedRoleBindings[i])
204
+			if err != nil {
205
+				return err
206
+			}
207
+
208
+			fmt.Fprintf(o.Out, "clusterrolebinding/%s\n", createdRoleBinding.Name)
209
+			continue
210
+		}
211
+
212
+		// RoleRef is immutable, to reset this, we have to delete/recreate
213
+		if !kapi.Semantic.DeepEqual(roleBinding.RoleRef, changedRoleBindings[i].RoleRef) {
214
+			roleBinding.RoleRef = changedRoleBindings[i].RoleRef
215
+			roleBinding.Subjects = changedRoleBindings[i].Subjects
216
+
217
+			// TODO: for extra credit, determine whether the right to delete/create this rolebinding for the current user came from this rolebinding before deleting it
218
+			err := o.RoleBindingClient.Delete(roleBinding.Name)
219
+			if err != nil {
220
+				return err
221
+			}
222
+			createdRoleBinding, err := o.RoleBindingClient.Create(changedRoleBindings[i])
223
+			if err != nil {
224
+				return err
225
+			}
226
+			fmt.Fprintf(o.Out, "clusterrolebinding/%s\n", createdRoleBinding.Name)
227
+			continue
228
+		}
229
+
230
+		roleBinding.Subjects = changedRoleBindings[i].Subjects
231
+		updatedRoleBinding, err := o.RoleBindingClient.Update(roleBinding)
232
+		if err != nil {
233
+			return err
234
+		}
235
+
236
+		fmt.Fprintf(o.Out, "clusterrolebinding/%s\n", updatedRoleBinding.Name)
237
+	}
238
+
239
+	return nil
240
+}
241
+
242
+func computeUpdatedBinding(expected authorizationapi.ClusterRoleBinding, actual authorizationapi.ClusterRoleBinding, excludeSubjects []kapi.ObjectReference, union bool) (*authorizationapi.ClusterRoleBinding, bool) {
243
+	needsUpdating := false
244
+
245
+	// Always reset the roleref if it is different
246
+	if !kapi.Semantic.DeepEqual(expected.RoleRef, actual.RoleRef) {
247
+		needsUpdating = true
248
+	}
249
+
250
+	// compute the list of subjects we should not add roles for (existing subjects in the exclude list should be preserved)
251
+	doNotAddSubjects, _ := diff(excludeSubjects, actual.Subjects)
252
+	// remove any excluded subjects that do not exist from our expected subject list (so we don't add them)
253
+	expectedSubjects, _ := diff(expected.Subjects, doNotAddSubjects)
254
+
255
+	missingSubjects, extraSubjects := diff(expectedSubjects, actual.Subjects)
256
+	// Always add missing expected subjects
257
+	if len(missingSubjects) > 0 {
258
+		needsUpdating = true
259
+	}
260
+	// extra subjects only require a change if we're not unioning
261
+	if len(extraSubjects) > 0 && !union {
262
+		needsUpdating = true
263
+	}
264
+
265
+	if !needsUpdating {
266
+		return nil, false
267
+	}
268
+
269
+	updated := expected
270
+	updated.Subjects = expectedSubjects
271
+	if union {
272
+		updated.Subjects = append(updated.Subjects, extraSubjects...)
273
+	}
274
+	return &updated, true
275
+}
276
+
277
+func contains(list []kapi.ObjectReference, item kapi.ObjectReference) bool {
278
+	for _, listItem := range list {
279
+		if kapi.Semantic.DeepEqual(listItem, item) {
280
+			return true
281
+		}
282
+	}
283
+	return false
284
+}
285
+
286
+// diff returns lists containing the items unique to each provided list:
287
+//   list1Only = list1 - list2
288
+//   list2Only = list2 - list1
289
+// if both returned lists are empty, the provided lists are equal
290
+func diff(list1 []kapi.ObjectReference, list2 []kapi.ObjectReference) (list1Only []kapi.ObjectReference, list2Only []kapi.ObjectReference) {
291
+	for _, list1Item := range list1 {
292
+		if !contains(list2, list1Item) {
293
+			if !contains(list1Only, list1Item) {
294
+				list1Only = append(list1Only, list1Item)
295
+			}
296
+		}
297
+	}
298
+	for _, list2Item := range list2 {
299
+		if !contains(list1, list2Item) {
300
+			if !contains(list2Only, list2Item) {
301
+				list2Only = append(list2Only, list2Item)
302
+			}
303
+		}
304
+	}
305
+	return
306
+}
0 307
new file mode 100644
... ...
@@ -0,0 +1,191 @@
0
+package policy
1
+
2
+import (
3
+	"testing"
4
+
5
+	kapi "k8s.io/kubernetes/pkg/api"
6
+
7
+	authorizationapi "github.com/openshift/origin/pkg/authorization/api"
8
+)
9
+
10
+func binding(roleRef kapi.ObjectReference, subjects []kapi.ObjectReference) *authorizationapi.ClusterRoleBinding {
11
+	return &authorizationapi.ClusterRoleBinding{RoleRef: roleRef, Subjects: subjects}
12
+}
13
+
14
+func ref(name string) kapi.ObjectReference {
15
+	return kapi.ObjectReference{Name: name}
16
+}
17
+
18
+func refs(names ...string) []kapi.ObjectReference {
19
+	r := []kapi.ObjectReference{}
20
+	for _, name := range names {
21
+		r = append(r, ref(name))
22
+	}
23
+	return r
24
+}
25
+
26
+func TestDiff(t *testing.T) {
27
+	tests := map[string]struct {
28
+		A             []kapi.ObjectReference
29
+		B             []kapi.ObjectReference
30
+		ExpectedOnlyA []kapi.ObjectReference
31
+		ExpectedOnlyB []kapi.ObjectReference
32
+	}{
33
+		"empty": {},
34
+
35
+		"matching, order-independent": {
36
+			A: refs("foo", "bar"),
37
+			B: refs("bar", "foo"),
38
+		},
39
+
40
+		"partial match": {
41
+			A:             refs("foo", "bar"),
42
+			B:             refs("foo", "baz"),
43
+			ExpectedOnlyA: refs("bar"),
44
+			ExpectedOnlyB: refs("baz"),
45
+		},
46
+
47
+		"missing": {
48
+			A:             refs("foo"),
49
+			B:             refs("bar"),
50
+			ExpectedOnlyA: refs("foo"),
51
+			ExpectedOnlyB: refs("bar"),
52
+		},
53
+
54
+		"remove duplicates": {
55
+			A:             refs("foo", "foo"),
56
+			B:             refs("bar", "bar"),
57
+			ExpectedOnlyA: refs("foo"),
58
+			ExpectedOnlyB: refs("bar"),
59
+		},
60
+	}
61
+
62
+	for k, tc := range tests {
63
+		onlyA, onlyB := diff(tc.A, tc.B)
64
+		if !kapi.Semantic.DeepEqual(onlyA, tc.ExpectedOnlyA) {
65
+			t.Errorf("%s: Expected %#v, got %#v", k, tc.ExpectedOnlyA, onlyA)
66
+		}
67
+		if !kapi.Semantic.DeepEqual(onlyB, tc.ExpectedOnlyB) {
68
+			t.Errorf("%s: Expected %#v, got %#v", k, tc.ExpectedOnlyB, onlyB)
69
+		}
70
+	}
71
+}
72
+
73
+func TestComputeUpdate(t *testing.T) {
74
+	tests := map[string]struct {
75
+		ExpectedBinding *authorizationapi.ClusterRoleBinding
76
+		ActualBinding   *authorizationapi.ClusterRoleBinding
77
+		ExcludeSubjects []kapi.ObjectReference
78
+		Union           bool
79
+
80
+		ExpectedUpdatedBinding *authorizationapi.ClusterRoleBinding
81
+		ExpectedUpdateNeeded   bool
82
+	}{
83
+		"match without union": {
84
+			ExpectedBinding: binding(ref("role"), refs("a")),
85
+			ActualBinding:   binding(ref("role"), refs("a")),
86
+			Union:           false,
87
+
88
+			ExpectedUpdatedBinding: nil,
89
+			ExpectedUpdateNeeded:   false,
90
+		},
91
+		"match with union": {
92
+			ExpectedBinding: binding(ref("role"), refs("a")),
93
+			ActualBinding:   binding(ref("role"), refs("a")),
94
+			Union:           true,
95
+
96
+			ExpectedUpdatedBinding: nil,
97
+			ExpectedUpdateNeeded:   false,
98
+		},
99
+
100
+		"different roleref with identical subjects": {
101
+			ExpectedBinding: binding(ref("role"), refs("a")),
102
+			ActualBinding:   binding(ref("differentRole"), refs("a")),
103
+			Union:           true,
104
+
105
+			ExpectedUpdatedBinding: binding(ref("role"), refs("a")),
106
+			ExpectedUpdateNeeded:   true,
107
+		},
108
+
109
+		"extra subjects without union": {
110
+			ExpectedBinding: binding(ref("role"), refs("a")),
111
+			ActualBinding:   binding(ref("role"), refs("a", "b")),
112
+			Union:           false,
113
+
114
+			ExpectedUpdatedBinding: binding(ref("role"), refs("a")),
115
+			ExpectedUpdateNeeded:   true,
116
+		},
117
+		"extra subjects with union": {
118
+			ExpectedBinding: binding(ref("role"), refs("a")),
119
+			ActualBinding:   binding(ref("role"), refs("a", "b")),
120
+			Union:           true,
121
+
122
+			ExpectedUpdatedBinding: nil,
123
+			ExpectedUpdateNeeded:   false,
124
+		},
125
+
126
+		"missing subjects without union": {
127
+			ExpectedBinding: binding(ref("role"), refs("a", "c")),
128
+			ActualBinding:   binding(ref("role"), refs("a", "b")),
129
+			Union:           false,
130
+
131
+			ExpectedUpdatedBinding: binding(ref("role"), refs("a", "c")),
132
+			ExpectedUpdateNeeded:   true,
133
+		},
134
+		"missing subjects with union": {
135
+			ExpectedBinding: binding(ref("role"), refs("a", "c")),
136
+			ActualBinding:   binding(ref("role"), refs("a", "b")),
137
+			Union:           true,
138
+
139
+			ExpectedUpdatedBinding: binding(ref("role"), refs("a", "c", "b")),
140
+			ExpectedUpdateNeeded:   true,
141
+		},
142
+
143
+		"do not add should make update unnecessary": {
144
+			ExpectedBinding: binding(ref("role"), refs("a", "b")),
145
+			ActualBinding:   binding(ref("role"), refs("a")),
146
+			ExcludeSubjects: refs("b"),
147
+			Union:           false,
148
+
149
+			ExpectedUpdatedBinding: nil,
150
+			ExpectedUpdateNeeded:   false,
151
+		},
152
+		"do not add should not add": {
153
+			ExpectedBinding: binding(ref("role"), refs("a", "b", "c")),
154
+			ActualBinding:   binding(ref("role"), refs("a")),
155
+			ExcludeSubjects: refs("c"),
156
+			Union:           false,
157
+
158
+			ExpectedUpdatedBinding: binding(ref("role"), refs("a", "b")),
159
+			ExpectedUpdateNeeded:   true,
160
+		},
161
+		"do not add should not remove": {
162
+			ExpectedBinding: binding(ref("role"), refs("a", "b")),
163
+			ActualBinding:   binding(ref("role"), refs("a", "b", "c")),
164
+			ExcludeSubjects: refs("b"),
165
+			Union:           false,
166
+
167
+			ExpectedUpdatedBinding: binding(ref("role"), refs("a", "b")),
168
+			ExpectedUpdateNeeded:   true,
169
+		},
170
+		"do not add complex": {
171
+			ExpectedBinding: binding(ref("role"), refs("matching1", "matching2", "missing1", "missing2")),
172
+			ActualBinding:   binding(ref("role"), refs("matching1", "matching2", "extra1", "extra2")),
173
+			ExcludeSubjects: refs("matching1", "missing1", "extra1"),
174
+			Union:           false,
175
+
176
+			ExpectedUpdatedBinding: binding(ref("role"), refs("matching1", "matching2", "missing2")),
177
+			ExpectedUpdateNeeded:   true,
178
+		},
179
+	}
180
+
181
+	for k, tc := range tests {
182
+		updatedBinding, updateNeeded := computeUpdatedBinding(*tc.ExpectedBinding, *tc.ActualBinding, tc.ExcludeSubjects, tc.Union)
183
+		if updateNeeded != tc.ExpectedUpdateNeeded {
184
+			t.Errorf("%s: Expected\n\t%v\ngot\n\t%v", k, tc.ExpectedUpdateNeeded, updateNeeded)
185
+		}
186
+		if !kapi.Semantic.DeepEqual(updatedBinding, tc.ExpectedUpdatedBinding) {
187
+			t.Errorf("%s: Expected\n\t%v %v\ngot\n\t%v %v", k, tc.ExpectedUpdatedBinding.RoleRef, tc.ExpectedUpdatedBinding.Subjects, updatedBinding.RoleRef, updatedBinding.Subjects)
188
+		}
189
+	}
190
+}
... ...
@@ -157,6 +157,12 @@ func (o *ReconcileClusterRolesOptions) ChangedClusterRoles() ([]*authorizationap
157 157
 			return nil, err
158 158
 		}
159 159
 
160
+		// Copy any existing labels/annotations, so the displayed update is correct
161
+		// This assumes bootstrap roles will not set any labels/annotations
162
+		// These aren't actually used during update; the latest labels/annotations are pulled from the existing object again
163
+		expectedClusterRole.Labels = actualClusterRole.Labels
164
+		expectedClusterRole.Annotations = actualClusterRole.Annotations
165
+
160 166
 		if !kapi.Semantic.DeepEqual(expectedClusterRole.Rules, actualClusterRole.Rules) {
161 167
 			if o.Union {
162 168
 				_, missingRules := rulevalidation.Covers(expectedClusterRole.Rules, actualClusterRole.Rules)
... ...
@@ -13,7 +13,8 @@ os::log::install_errexit
13 13
   set +e
14 14
   oc delete project/example project/ui-test-project project/recreated-project
15 15
   oc delete sa/router -n default
16
-  oadm policy reconcile-cluster-roles
16
+  oadm policy reconcile-cluster-roles --confirm
17
+  oadm policy reconcile-cluster-role-bindings --confirm
17 18
 ) 2>/dev/null 1>&2
18 19
 
19 20
 defaultimage="openshift/origin-\${component}:latest"
... ...
@@ -75,6 +76,7 @@ oadm policy add-cluster-role-to-group cluster-admin system:unauthenticated
75 75
 oadm policy remove-cluster-role-from-group cluster-admin system:unauthenticated
76 76
 oadm policy add-cluster-role-to-user cluster-admin system:no-user
77 77
 oadm policy remove-cluster-role-from-user cluster-admin system:no-user
78
+
78 79
 oc delete clusterrole/cluster-status
79 80
 [ ! "$(oc get clusterrole/cluster-status)" ]
80 81
 oadm policy reconcile-cluster-roles
... ...
@@ -82,10 +84,40 @@ oadm policy reconcile-cluster-roles
82 82
 oadm policy reconcile-cluster-roles --confirm
83 83
 oc get clusterrole/cluster-status
84 84
 oc replace --force -f ./test/fixtures/basic-user.json
85
+# display shows customized labels/annotations
86
+[ "$(oadm policy reconcile-cluster-roles | grep custom-label)" ]
87
+[ "$(oadm policy reconcile-cluster-roles | grep custom-annotation)" ]
85 88
 oadm policy reconcile-cluster-roles --additive-only --confirm
89
+# reconcile preserves added rules, labels, and annotations
90
+[ "$(oc get clusterroles/basic-user -o json | grep custom-label)" ]
91
+[ "$(oc get clusterroles/basic-user -o json | grep custom-annotation)" ]
86 92
 [ "$(oc get clusterroles/basic-user -o json | grep groups)" ]
87 93
 oadm policy reconcile-cluster-roles --confirm
88 94
 [ ! "$(oc get clusterroles/basic-user -o json | grep groups)" ]
95
+
96
+# Ensure a removed binding gets re-added
97
+oc delete clusterrolebinding/cluster-status-binding
98
+[ ! "$(oc get clusterrolebinding/cluster-status-binding)" ]
99
+oadm policy reconcile-cluster-role-bindings
100
+[ ! "$(oc get clusterrolebinding/cluster-status-binding)" ]
101
+oadm policy reconcile-cluster-role-bindings --confirm
102
+oc get clusterrolebinding/cluster-status-binding
103
+# Customize a binding
104
+oc replace --force -f ./test/fixtures/basic-users-binding.json
105
+# display shows customized labels/annotations
106
+[ "$(oadm policy reconcile-cluster-role-bindings | grep custom-label)" ]
107
+[ "$(oadm policy reconcile-cluster-role-bindings | grep custom-annotation)" ]
108
+oadm policy reconcile-cluster-role-bindings --confirm
109
+# Ensure a customized binding's subjects, labels, annotations are retained by default
110
+[ "$(oc get clusterrolebindings/basic-users -o json | grep custom-label)" ]
111
+[ "$(oc get clusterrolebindings/basic-users -o json | grep custom-annotation)" ]
112
+[ "$(oc get clusterrolebindings/basic-users -o json | grep custom-user)" ]
113
+# Ensure a customized binding's roleref is corrected
114
+[ ! "$(oc get clusterrolebindings/basic-users -o json | grep cluster-status)" ]
115
+# Ensure --additive-only=false removes customized users from the binding
116
+oadm policy reconcile-cluster-role-bindings --additive-only=false --confirm
117
+[ ! "$(oc get clusterrolebindings/basic-users -o json | grep custom-user)" ]
118
+
89 119
 echo "admin-policy: ok"
90 120
 
91 121
 # Test the commands the UI projects page tells users to run
... ...
@@ -6,7 +6,13 @@
6 6
         "selfLink": "/osapi/v1beta3/clusterroles/basic-user",
7 7
         "uid": "86a5ce7c-3f44-11e5-a9a6-080027c5bfa9",
8 8
         "resourceVersion": "18",
9
-        "creationTimestamp": "2015-08-10T09:45:25Z"
9
+        "creationTimestamp": "2015-08-10T09:45:25Z",
10
+        "labels": {
11
+        	"custom-label":"value"
12
+        },
13
+        "annotations": {
14
+        	"custom-annotation":"value"
15
+        }
10 16
     },
11 17
     "rules": [
12 18
         {
13 19
new file mode 100644
... ...
@@ -0,0 +1,30 @@
0
+{
1
+    "kind": "ClusterRoleBinding",
2
+    "apiVersion": "v1",
3
+    "metadata": {
4
+        "name": "basic-users",
5
+        "selfLink": "/oapi/v1/clusterrolebindings/basic-users",
6
+        "uid": "8dcfc4cd-6917-11e5-8b72-3c970e4b7ffe",
7
+        "resourceVersion": "42",
8
+        "creationTimestamp": "2015-10-02T15:09:18Z",
9
+        "labels": {
10
+        	"custom-label":"value"
11
+        },
12
+        "annotations": {
13
+        	"custom-annotation":"value"
14
+        }
15
+    },
16
+    "subjects": [
17
+        {
18
+            "kind": "SystemGroup",
19
+            "name": "system:authenticated"
20
+        },
21
+        {
22
+            "kind": "User",
23
+            "name": "custom-user"
24
+        }
25
+    ],
26
+    "roleRef": {
27
+        "name": "cluster-status"
28
+    }
29
+}