From 0e5da1337dae91bb20b0dcfeca5104fb2dac6e12 Mon Sep 17 00:00:00 2001 From: David Eads Date: Mon, 2 Jul 2018 11:49:45 -0400 Subject: [PATCH 1/3] UPSTREAM: revert: : make auth reconcile work with backlevel versions until ansible updates This reverts commit 979704ac34b42f25827c8fbaf9040904bca82eb1. --- .../kubernetes/pkg/kubectl/cmd/auth/reconcile.go | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/vendor/k8s.io/kubernetes/pkg/kubectl/cmd/auth/reconcile.go b/vendor/k8s.io/kubernetes/pkg/kubectl/cmd/auth/reconcile.go index f4e84e31e84b..3346812deef5 100644 --- a/vendor/k8s.io/kubernetes/pkg/kubectl/cmd/auth/reconcile.go +++ b/vendor/k8s.io/kubernetes/pkg/kubectl/cmd/auth/reconcile.go @@ -25,7 +25,6 @@ import ( rbacv1 "k8s.io/api/rbac/v1" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" rbacv1client "k8s.io/client-go/kubernetes/typed/rbac/v1" - "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/kubectl/cmd/templates" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" "k8s.io/kubernetes/pkg/kubectl/genericclioptions" @@ -106,7 +105,7 @@ func (o *ReconcileOptions) Complete(cmd *cobra.Command, f cmdutil.Factory, args } r := f.NewBuilder(). - WithScheme(legacyscheme.Scheme). + WithScheme(scheme.Scheme, scheme.Scheme.PrioritizedVersionsAllGroups()...). ContinueOnError(). NamespaceParam(namespace).DefaultNamespace(). FilenameParam(enforceNamespace, o.FilenameOptions). @@ -171,14 +170,7 @@ func (o *ReconcileOptions) RunReconcile() error { return err } - obj, err := legacyscheme.Scheme.ConvertToVersion(info.Object, rbacv1.SchemeGroupVersion) - if err != nil { - glog.V(1).Infof("skipping %#v", info.Object.GetObjectKind()) - // skip ignored resources - return nil - } - - switch t := obj.(type) { + switch t := info.Object.(type) { case *rbacv1.Role: reconcileOptions := reconciliation.ReconcileRoleOptions{ Confirm: !o.DryRun, From b10515ad58bda69b033bfdc1fd462e03448e4a7c Mon Sep 17 00:00:00 2001 From: David Eads Date: Thu, 5 Jul 2018 13:14:59 -0400 Subject: [PATCH 2/3] UPSTREAM: 65715: fail on rbac resources of non-v1 versions in reconcile --- .../hack/make-rules/test-cmd-util.sh | 3 +++ .../pkg/kubectl/cmd/auth/reconcile.go | 13 ++++++++++ .../pkg/kubectl/cmd/auth/rbac-v1beta1.yaml | 25 +++++++++++++++++++ 3 files changed, 41 insertions(+) create mode 100644 vendor/k8s.io/kubernetes/test/fixtures/pkg/kubectl/cmd/auth/rbac-v1beta1.yaml diff --git a/vendor/k8s.io/kubernetes/hack/make-rules/test-cmd-util.sh b/vendor/k8s.io/kubernetes/hack/make-rules/test-cmd-util.sh index 91f08751d679..bb8c63d74116 100755 --- a/vendor/k8s.io/kubernetes/hack/make-rules/test-cmd-util.sh +++ b/vendor/k8s.io/kubernetes/hack/make-rules/test-cmd-util.sh @@ -5335,6 +5335,9 @@ runTests() { kube::test::get_object_assert 'clusterrolebindings -l test-cmd=auth' "{{range.items}}{{$id_field}}:{{end}}" 'testing-CRB:' kube::test::get_object_assert 'clusterroles -l test-cmd=auth' "{{range.items}}{{$id_field}}:{{end}}" 'testing-CR:' + failure_message=$(! kubectl auth reconcile "${kube_flags[@]}" -f test/fixtures/pkg/kubectl/cmd/auth/rbac-v1beta1.yaml 2>&1 ) + kube::test::if_has_string "${failure_message}" 'only rbac.authorization.k8s.io/v1 is supported' + kubectl delete "${kube_flags[@]}" rolebindings,role,clusterroles,clusterrolebindings -n some-other-random -l test-cmd=auth fi diff --git a/vendor/k8s.io/kubernetes/pkg/kubectl/cmd/auth/reconcile.go b/vendor/k8s.io/kubernetes/pkg/kubectl/cmd/auth/reconcile.go index 3346812deef5..a30fb101ed83 100644 --- a/vendor/k8s.io/kubernetes/pkg/kubectl/cmd/auth/reconcile.go +++ b/vendor/k8s.io/kubernetes/pkg/kubectl/cmd/auth/reconcile.go @@ -23,6 +23,8 @@ import ( "github.com/spf13/cobra" rbacv1 "k8s.io/api/rbac/v1" + rbacv1beta1 "k8s.io/api/rbac/v1beta1" + rbacv1alpha1 "k8s.io/api/rbac/v1alpha1" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" rbacv1client "k8s.io/client-go/kubernetes/typed/rbac/v1" "k8s.io/kubernetes/pkg/kubectl/cmd/templates" @@ -32,6 +34,7 @@ import ( "k8s.io/kubernetes/pkg/kubectl/genericclioptions/resource" "k8s.io/kubernetes/pkg/kubectl/scheme" "k8s.io/kubernetes/pkg/registry/rbac/reconciliation" + "fmt" ) // ReconcileOptions is the start of the data required to perform the operation. As new fields are added, add them here instead of @@ -233,6 +236,16 @@ func (o *ReconcileOptions) RunReconcile() error { } o.PrintObject(result.RoleBinding.GetObject(), o.Out) + case *rbacv1beta1.Role, + *rbacv1beta1.RoleBinding, + *rbacv1beta1.ClusterRole, + *rbacv1beta1.ClusterRoleBinding, + *rbacv1alpha1.Role, + *rbacv1alpha1.RoleBinding, + *rbacv1alpha1.ClusterRole, + *rbacv1alpha1.ClusterRoleBinding: + return fmt.Errorf("only rbac.authorization.k8s.io/v1 is supported: not %T", t) + default: glog.V(1).Infof("skipping %#v", info.Object.GetObjectKind()) // skip ignored resources diff --git a/vendor/k8s.io/kubernetes/test/fixtures/pkg/kubectl/cmd/auth/rbac-v1beta1.yaml b/vendor/k8s.io/kubernetes/test/fixtures/pkg/kubectl/cmd/auth/rbac-v1beta1.yaml new file mode 100644 index 000000000000..2fb1f1fbbae0 --- /dev/null +++ b/vendor/k8s.io/kubernetes/test/fixtures/pkg/kubectl/cmd/auth/rbac-v1beta1.yaml @@ -0,0 +1,25 @@ +apiVersion: v1 +items: +- apiVersion: rbac.authorization.k8s.io/v1beta1 + kind: ClusterRole + metadata: + labels: + test-cmd: auth + name: testing-CR + rules: + - apiGroups: + - "" + resources: + - pods + verbs: + - create + - delete + - deletecollection + - get + - list + - patch + - update + - watch + +kind: List +metadata: {} From a9b4aeea853429d90c13be4add28c95323d29598 Mon Sep 17 00:00:00 2001 From: David Eads Date: Tue, 10 Jul 2018 13:48:46 -0400 Subject: [PATCH 3/3] simplify vendor commit checker --- tools/rebasehelpers/commitchecker/validate.go | 10 ++--- .../commitchecker/validate_test.go | 42 ++++++++++--------- tools/rebasehelpers/util/git.go | 2 +- 3 files changed, 26 insertions(+), 28 deletions(-) diff --git a/tools/rebasehelpers/commitchecker/validate.go b/tools/rebasehelpers/commitchecker/validate.go index 66c981d56823..cf7518704a0b 100644 --- a/tools/rebasehelpers/commitchecker/validate.go +++ b/tools/rebasehelpers/commitchecker/validate.go @@ -18,11 +18,11 @@ The following UPSTREAM commits have invalid summaries: {{ end }} UPSTREAM commit summaries should look like: - UPSTREAM: [non-kube-repo/name: ]: description + UPSTREAM: : description UPSTREAM commits which revert previous UPSTREAM commits should look like: - UPSTREAM: revert: : + UPSTREAM: revert: UPSTREAM commits are validated against the following regular expression: @@ -31,20 +31,16 @@ UPSTREAM commits are validated against the following regular expression: Examples of valid summaries: UPSTREAM: 12345: A kube fix - UPSTREAM: coreos/etcd: 12345: An etcd fix UPSTREAM: : A carried kube change UPSTREAM: : A dropped kube change - UPSTREAM: revert: abcd123: coreos/etcd: 12345: An etcd fix - UPSTREAM: k8s.io/heapster: 12345: A heapster fix + UPSTREAM: revert: 12345: A kube revert ` var AllValidators = []func([]util.Commit) error{ ValidateUpstreamCommitSummaries, ValidateUpstreamCommitsWithoutGodepsChanges, - ValidateUpstreamCommitModifiesSingleGodepsRepo, ValidateUpstreamCommitModifiesOnlyGodeps, - ValidateUpstreamCommitModifiesOnlyDeclaredGodepRepo, ValidateUpstreamCommitModifiesOnlyKubernetes, } diff --git a/tools/rebasehelpers/commitchecker/validate_test.go b/tools/rebasehelpers/commitchecker/validate_test.go index d82dc36832c3..812fcb722bd1 100644 --- a/tools/rebasehelpers/commitchecker/validate_test.go +++ b/tools/rebasehelpers/commitchecker/validate_test.go @@ -296,29 +296,31 @@ func TestValidateUpstreamCommitSummaries(t *testing.T) { {valid: true, summary: "UPSTREAM: : a change"}, {valid: true, summary: "UPSTREAM: coreos/etcd: : a change"}, {valid: true, summary: "UPSTREAM: coreos/etcd: : a change"}, - {valid: true, summary: "UPSTREAM: revert: abcd123: 12345: a change"}, - {valid: true, summary: "UPSTREAM: revert: abcd123: k8s.io/heapster: 12345: a change"}, - {valid: true, summary: "UPSTREAM: revert: abcd123: : a change"}, - {valid: true, summary: "UPSTREAM: revert: abcd123: : a change"}, - {valid: true, summary: "UPSTREAM: revert: abcd123: coreos/etcd: : a change"}, - {valid: true, summary: "UPSTREAM: revert: abcd123: coreos/etcd: : a change"}, + {valid: true, summary: "UPSTREAM: revert: 12345: a change"}, + {valid: true, summary: "UPSTREAM: revert: k8s.io/heapster: 12345: a change"}, + {valid: true, summary: "UPSTREAM: revert: : a change"}, + {valid: true, summary: "UPSTREAM: revert: : a change"}, + {valid: true, summary: "UPSTREAM: revert: coreos/etcd: : a change"}, + {valid: true, summary: "UPSTREAM: revert: coreos/etcd: : a change"}, {valid: false, summary: "UPSTREAM: whoopsie daisy"}, {valid: true, summary: "UPSTREAM: gopkg.in/ldap.v2: 51: exposed better API for paged search"}, } for _, test := range tests { - commit := util.Commit{Summary: test.summary, Sha: "abcd000"} - err := ValidateUpstreamCommitSummaries([]util.Commit{commit}) - if err != nil { - if test.valid { - t.Fatalf("unexpected error:\n%s", err) + t.Run(test.summary, func(t *testing.T) { + commit := util.Commit{Summary: test.summary, Sha: "abcd000"} + err := ValidateUpstreamCommitSummaries([]util.Commit{commit}) + if err != nil { + if test.valid { + t.Fatalf("unexpected error:\n%s", err) + } else { + t.Logf("got expected error:\n%s", err) + } } else { - t.Logf("got expected error:\n%s", err) + if !test.valid { + t.Fatalf("expected an error, got none; summary: %s", test.summary) + } } - } else { - if !test.valid { - t.Fatalf("expected an error, got none; summary: %s", test.summary) - } - } + }) } } @@ -409,7 +411,7 @@ func TestValidateUpstreamCommitModifiesOnlyDeclaredGodepRepo(t *testing.T) { commits: []util.Commit{ { Sha: "aaa0001", - Summary: "UPSTREAM: revert: abcd000: 12345: a change", + Summary: "UPSTREAM: revert: 12345: a change", Files: []util.File{ "Godeps/_workspace/src/k8s.io/kubernetes/file1", "Godeps/_workspace/src/k8s.io/kubernetes/file2", @@ -423,7 +425,7 @@ func TestValidateUpstreamCommitModifiesOnlyDeclaredGodepRepo(t *testing.T) { commits: []util.Commit{ { Sha: "aaa0001", - Summary: "UPSTREAM: revert: abcd000: coreos/etcd: 12345: a change", + Summary: "UPSTREAM: revert: coreos/etcd: 12345: a change", Files: []util.File{ "Godeps/_workspace/src/k8s.io/kubernetes/file1", "Godeps/_workspace/src/k8s.io/kubernetes/file2", @@ -437,7 +439,7 @@ func TestValidateUpstreamCommitModifiesOnlyDeclaredGodepRepo(t *testing.T) { commits: []util.Commit{ { Sha: "aaa0001", - Summary: "UPSTREAM: revert: abcd000: coreos/etcd: 12345: a change", + Summary: "UPSTREAM: revert: coreos/etcd: 12345: a change", Files: []util.File{ "Godeps/_workspace/src/github.com/coreos/etcd/file1", "Godeps/_workspace/src/github.com/coreos/etcd/file2", diff --git a/tools/rebasehelpers/util/git.go b/tools/rebasehelpers/util/git.go index f1c6bc06829d..42d3f77b7c05 100644 --- a/tools/rebasehelpers/util/git.go +++ b/tools/rebasehelpers/util/git.go @@ -9,7 +9,7 @@ import ( "strings" ) -var UpstreamSummaryPattern = regexp.MustCompile(`UPSTREAM: (revert: [a-f0-9]{7,}: )?(([\w\.-]+\/[\w-\.-]+)?: )?(\d+:|:|:)`) +var UpstreamSummaryPattern = regexp.MustCompile(`UPSTREAM: (revert: )?(([\w\.-]+\/[\w-\.-]+)?: )?(\d+:|:|:)`) // supportedHosts maps source hosts to the number of path segments that // represent the account/repo for that host. This is necessary because we