Skip to content

Updated v1alpha to use fine grain RBAC rules#272

Merged
rshriram merged 3 commits intoistio:masterfrom
dcberg:271
May 16, 2017
Merged

Updated v1alpha to use fine grain RBAC rules#272
rshriram merged 3 commits intoistio:masterfrom
dcberg:271

Conversation

@dcberg
Copy link
Copy Markdown
Contributor

@dcberg dcberg commented May 15, 2017

This is the fix for issue #217 .

Note, I had to add the "namespaces" resource type to the istio-manager ClusterRole. I'm guessing that the istio-rbac-beta.yaml has the same issue.


This change is Reviewable

@googlebot
Copy link
Copy Markdown
Collaborator

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@googlebot googlebot added the cla: no Set by the Google CLA bot to indicate the author of a PR has not signed the Google CLA. label May 15, 2017
@rshriram rshriram requested a review from costinm May 15, 2017 02:50
@rshriram rshriram added cla: yes and removed cla: no Set by the Google CLA bot to indicate the author of a PR has not signed the Google CLA. labels May 15, 2017
@rshriram
Copy link
Copy Markdown
Member

@costinm @andraxylia can you guys confirm if this is okay?

# If using minikube, start with '--extra-config=apiserver.Authorization.Mode=RBAC'
#
# NOTE: If deploying istio to a namespace other than 'default' then change the
# ClusterRoleBinding namspace target appropriately.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If you change ClusterRoleBinding to simply RoleBinding, then you don't need to specify namespace. The binding will be scoped to whatever namespace kubectl is using.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So, keep in mind that @dcberg tweaked the existing RBAC beta to work on RBAC v1alpha1. This is what he got working. So, before we suggest alternatives, we need to make sure that this would work on v1alpha1. I remember trying the RoleBinding and it didn't work (IIRC).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In v1aplha1 RoleBinding does not work with ClusterRoles.

resources: ["thirdpartyresources", "thirdpartyresources.extensions", "ingresses"]
verbs: ["*"]
- apiGroups: [""]
resources: ["configmaps", "endpoints", "pods", "services", "namespaces"]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are you letting the manager to listen on all namespaces? Otherwise the manager don't need access to namespace objects.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The problem I hit without adding "namespaces" was that istioctl requests would fail because the manager was trying to list namespaces and this was failing due to rbac.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

istioctl create -f samples/apps/bookinfo/route-rule-all-v1.yaml Error: an error on the server ("namespace \"istio\" not present") has prevented the request from succeeding

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

2017-05-15T18:23:14.640098266Z I0515 18:23:14.639846 1 config.go:41] Parsed route-rule productpage-default into istio.proxy.v1.config.RouteRule destination:"productpage.istio.svc.cluster.local" precedence:1 route:<tags:<key:"version" value:"v1" > > 2017-05-15T18:23:14.640140163Z I0515 18:23:14.639899 1 handler.go:78] Adding config to Istio registry: key istio/route-rule-productpage-default, config &{Type:route-rule Name:productpage-default Spec:map[route:[map[tags:map[version:v1]]] destination:productpage.istio.svc.cluster.local precedence:1] ParsedSpec:destination:"productpage.istio.svc.cluster.local" precedence:1 route:<tags:<key:"version" value:"v1" > > } 2017-05-15T18:23:14.642366289Z W0515 18:23:14.642132 1 handler.go:205] namespace "istio" not present

@dcberg
Copy link
Copy Markdown
Contributor Author

dcberg commented May 15, 2017

I've signed the CLA.
Also working under IBM as well.

@googlebot
Copy link
Copy Markdown
Collaborator

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

@googlebot googlebot added cla: no Set by the Google CLA bot to indicate the author of a PR has not signed the Google CLA. and removed cla: yes labels May 15, 2017
@rshriram rshriram added cla: yes and removed cla: no Set by the Google CLA bot to indicate the author of a PR has not signed the Google CLA. labels May 16, 2017
@rshriram
Copy link
Copy Markdown
Member

jenkins rebuild istio/presubmit

@rshriram rshriram merged commit 736157a into istio:master May 16, 2017
@dcberg
Copy link
Copy Markdown
Contributor Author

dcberg commented May 16, 2017

Updated the PR to include the "update" verb for the istio-ca ClusterRole for "secrets" based on failures that I was seeing in the istio-ca pod.
2017-05-15T23:14:06.807213270Z E0515 23:14:06.807016 1 secret.go:244] Failed to update secret istio/istio.default (error: Forbidden: "/api/v1/namespaces/istio/secrets/istio.default" (put secrets istio.default))

I also refined the "istio-manager" role to only have "get" and "list" access on "namespaces"

zenlint pushed a commit to zenlint/istio that referenced this pull request Aug 30, 2017
rshriram pushed a commit that referenced this pull request Oct 30, 2017
* Updated v1alpha to use fine grain RBAC rules

* updated alpha rbac to add update support to istio-ca

* Improve rbac for istio-ca and istio-manager


Former-commit-id: 736157a
mandarjog pushed a commit that referenced this pull request Nov 2, 2017
* Updated v1alpha to use fine grain RBAC rules

* updated alpha rbac to add update support to istio-ca

* Improve rbac for istio-ca and istio-manager


Former-commit-id: 736157a
howardjohn pushed a commit to howardjohn/istio that referenced this pull request Jan 12, 2020
howardjohn pushed a commit to howardjohn/istio that referenced this pull request Jan 12, 2020
* use values proto

* Fix more fields

* Fix some more fields, adds todo

* Add more fields

* Add more fields

* Address comments

* Try to fix enum issue

* Use golang protobuf

* Address lint issue

* Update golden files

* Address review comment

* rebase

* Address new comments

* Rebase
luksa pushed a commit to luksa/istio that referenced this pull request Apr 26, 2021
* MAISTRA-1475: Don't use pre-compiled WASM extensions

We are not going to use them for 2.1, because they are built
on x86 architecture, hence they will not work on other arches.

So, let's use normal, arch-independent wasm files.

* make gen
luksa pushed a commit to luksa/istio that referenced this pull request Jun 30, 2021
* MAISTRA-1475: Don't use pre-compiled WASM extensions

We are not going to use them for 2.1, because they are built
on x86 architecture, hence they will not work on other arches.

So, let's use normal, arch-independent wasm files.

* make gen
luksa pushed a commit to luksa/istio that referenced this pull request Feb 22, 2022
* MAISTRA-1475: Don't use pre-compiled WASM extensions

We are not going to use them for 2.1, because they are built
on x86 architecture, hence they will not work on other arches.

So, let's use normal, arch-independent wasm files.

* make gen
luksa pushed a commit to luksa/istio that referenced this pull request Apr 29, 2022
* MAISTRA-1475: Don't use pre-compiled WASM extensions

We are not going to use them for 2.1, because they are built
on x86 architecture, hence they will not work on other arches.

So, let's use normal, arch-independent wasm files.

* make gen
aniketsingh03 pushed a commit to aniketsingh03/istio-1 that referenced this pull request Oct 4, 2022
…nt from `Proxy.IPAddresses` (istio#150) (istio#172) (istio#218) (istio#272)

Signed-off-by: Yaroslav Skopets <yaroslav@tetrate.io>
fjglira pushed a commit to fjglira/istio that referenced this pull request Feb 10, 2025
…ter-merge_upstream_istio_master-6253864e

Automator: merge upstream changes to openshift-service-mesh/istio@master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants