Moved RBAC CRDs to group rbac.istio.io from config.istio.io#6874
Moved RBAC CRDs to group rbac.istio.io from config.istio.io#6874rshriram merged 16 commits intoistio:release-1.0from ymesika:rbacRefactor_rel_1.0
Conversation
Codecov Report
@@ Coverage Diff @@
## release-1.0 #6874 +/- ##
============================================
- Coverage 72% 72% -<1%
============================================
Files 356 356
Lines 30580 30493 -87
============================================
- Hits 21875 21777 -98
- Misses 7791 7797 +6
- Partials 914 919 +5
Continue to review full report at Codecov.
|
|
Is there a migration story for deployments with existing resources? |
|
@rshriram that's a good point |
|
An approach like this can help support old and new API Groups side-by-side: This is not tested, and still not complete enough to help with the RBAC case. For that, I think the right model would be to pass the full APIGroup/Version/Kind information instead of simply kinds. |
| - httpapispecbindings | ||
| - quotaspecs | ||
| - quotaspecbindings | ||
| - operations: |
There was a problem hiding this comment.
We can validate all kinds/versions once RBAC is in its own group (see below). The version/kind specific qualification was only required for config.istio.io which is split across mixer and pilot validation.
- operations:
- CREATE
- UPDATE
apiGroups:
- rbac.istio.io
apiVersions:
- "*"
resources:
- "*"| verbs: ["create", "get", "list", "watch", "patch"] | ||
| - apiGroups: ["rbac.istio.io"] # istio RBAC watcher | ||
| resources: ["*"] | ||
| verbs: ["create", "get", "list", "watch", "patch"] |
There was a problem hiding this comment.
Why does mixer need to create and patch RBAC resources?
| verbs: ["*"] | ||
| - apiGroups: ["rbac.istio.io"] | ||
| resources: ["*"] | ||
| verbs: ["*"] |
There was a problem hiding this comment.
Pilot should need read-only access (i.e. get, watch, and list verbs)
|
/hold We should not check this in until the migration/compatibility story is addressed. |
|
Are there any other resources that needs to be migrated when upgrading from 0.8 to 1.0? Is there a general story for an upgrade or at the moment everything in 1.0 is compatible with 0.8? |
|
Replacing
#1 is a non-starter if people are using RBAC is production IMO. #2 gets the job done in one release cycle but requires more complex installer. #3 essentially does #2 but over two release cycles and doesn't require the installer. Regardless of #2 or #3, we need to update references/samples/examples, mention it in the release notes, and consider creating a conversion command (analogous to convert-networking-config). |
|
Thanks for making this change, I'm a little confused about why do we need this rename? Is there any tech reason or is this just a naming convention for 1.0? cc @liminw |
| Type: "service-role-binding", | ||
| Plural: "service-role-bindings", | ||
| Group: "config", | ||
| Group: "rbac", |
There was a problem hiding this comment.
Could you also change the group name of RbacConfig in line 547? It's used to configure the global RBAC policy.
|
/hold Do we really need this change at this late stage in the release? Can we just live with the old name for 1.0 and fix for 1.1? |
|
RBAC is unused so far in production. So I am not worried about migration, if and only if RBAC is the only thing sitting under config.istio.io. The name is extremely confusing. Given that we already have 20+ api objects, it’s very important to have a name that distinguishes the system over a generic “config.istio.io”. It’s also better to do this now, when no one is using it in production than postponing this by 3 months when there is a higher probability of someone using this in production and demanding backward compatibility. |
|
I am not that worried about migration. RBAC is currently in Alpha. The earlier we make the change, the better. |
|
@yangminzhu / @liminw is this okay to merge? if so, please approve the PR |
|
/hold cancel |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: yangminzhu, ymesika Assign the PR to them by writing The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
* Remove api.* attributes from accesslog as they are yet to be implemented (#7019) * mixer: setup initial liveness probe (#7065) * Setup initial liveness probe for Mixer * Prefer /version to /metrics * remove unused legacy v1alpha1 routing apis (#7091) * Updating Kiali to v0.5.0 in Helm Installer (#7007) * Updating Kiali to v0.5.0 * Adding quotaspecs and quotaspecbindings in clusterole * Upgrading Kiali Versrion on Requirements File * better logging for attribute bag (#7031) Signed-off-by: Kuat Yessenov <kuat@google.com> * Istio-remote helm: allow setup pilot svc & endpoint for controlPlaneSecurity (#7094) When controlPlaneSecurityEnabled the spiffe configured by sidecar injector assumes that the pilot discoveryAddress is a hostname.namespace formatted string. This change makes the istio-remote helm chart setup a selectorless service and endpoint from the pilotAddress when the remotePilotCreateSvcEndpoint arg is true and sets the MeshConfig discoveryAddress to be the hostname for istio-pilot. * remove MeshPolicy/DefaultDestRule when mtls isn't enabled (#7095) * Change job name to indicate release version (#7092) * Polishing for messages, reporting, metrics (#6946) * Make version, timestamp available in service for k8s * More error and metrics cleanup * Address Shriram's comments * Format * Fix verbose log, format, init * More work - the push is async, and things are trickier than they look * Rename mixer job, so upgrade works * Add back iperf3 without sidecar * Format * Switch to *env, no copy by value * Cleanup, resolve conflicts * Also convert node to pointer, and add explicit param * Polish, cleanups * Revert changes moved to other PRs * Logging too much * Polish, less log * Extra safety, sometimes a stale message is stuck until config change * fmt and lint errors * Addres review comments * Add the ttls test * Revert the review fix - it breaks tests (some send old style). Will update post 1.1 * Fix for ingress conflict with tracing ingress (#7039) * force to use proxyv2 for istio-telemetry pod (#7111) * delete unix socket, before calling listen (#7110) * typo: connctionDuration -> connectionDuration (#7134) * Add RbacConfig CRD to Helm chart (#7125) * Fix two data races (#7119) * TCP routing cleanups/bug fixes: part 1 (#7101) * TCP routing cleanups: part 1 Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * missing sort Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * sort destination cidrs * proxy sha update * undo istio proxy sha * fixups * compile fixes * nil check Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * nits Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * Update proxy sha based on Piotrs changes * undo Piotrs changes * add reporterKind attribute to accesslog and tcpaccesslog (#7147) * Improve outbound listener conflict messages (#7113) This is needed to better support e2e testing of conflicts. * Update accesslog config (#7158) * Update hub and tag in values.yaml in use daily releases (#7116) * add latest tag to values.yaml * switch latest in istioctl * configurable mixer port (#7156) Signed-off-by: Kuat Yessenov <kuat@google.com> * Adding unit test for TCP over TCP conflicts. (#7160) * Adding unit test for TCP over TCP conflicts. * addressing comments. * addressing comments. * addressing comments. * Assorted small fixes for setup (#7123) * Assorted small fixes * spaces * Fix certmanager (#7141) * Adding e2e test for outbound listener conflicts (#7173) * remove static route generation. improve gateway validation (#7175) * remove static route generation Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * more validation for gateways Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * validation fixes Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * fix test Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * Updating Kiali to v0.5.0 in Ansible Installer (#7042) * Multicluster make zipkin conditional (#7085) This change makes the zipkin arguement in the sidecar config map to be conditional based on zipkin being used in remote clusters. * Yet another place of the connctionDuration typo (#7177) * Put back citadel templates and re-generate them when running updateVersion.sh (#7178) * make bookinfo sample build repeatable (#7152) * make bookinfo sample build repeatable * update bookinfo image references to 1.8 * delete me later-- reference temporary images * remove v1 registration from readiness probe and use debu/endpointz (#7189) * bookinfo fixes (#7109) * update bookinfo productpage service * update bookinfo reviews service * update routing rules * fix me after-- update image references * update e2e tests * fix buildinfo build bug from #6988 * Cleaning up inject test files (#7188) There are a lot of files and its confusing which files are used by inject_test vs webhook_test. Splitting out the files into two subdirectories under testdata/ to make it clear. Making copies of files that are shared. * add TCP mixerfilter test (#7172) Signed-off-by: Kuat Yessenov <kuat@google.com> * Fix broken ingress (#7144) * Fix ingress * Move merging ingress into VirtualService and dedup to separate PR. Add back the previously and still broken gateway generation * Another attempt to avoid duplicated vhosts and merge ingress * Missed a pointer * Remove again mergeIngress * Improved messages. Will need to be turned into metrics soon * Merge, assorted fixes * Leave the push param in gateay, it will be needed later * deps: update proxy (#7196) * update sha Signed-off-by: Kuat Yessenov <kuat@google.com> * merge Signed-off-by: Kuat Yessenov <kuat@google.com> * fix tests Signed-off-by: Kuat Yessenov <kuat@google.com> * typo Signed-off-by: Kuat Yessenov <kuat@google.com> * rbac: add e2e tests for envoy based rbac. (#7193) See #6377. * Cleanup default metrics configuration (#7034) * WIP: Cleanup default metrics configuration * Remove last vestige of destination_namespace * Update accesslog config * Update syntax used to configure xDS servers. (#7201) Signed-off-by: Piotr Sikora <piotrsikora@google.com> * Update go-control-plane to 8cef83f9. (#7202) Signed-off-by: Piotr Sikora <piotrsikora@google.com> * bump bookinfo version (#7195) Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * Add contents of install/ansible to release script (#7194) Fixes: #4359, istio/old_issues_repo#223 * update for 1.0.0 path (#7088) * update api sha (#7204) * Delete namespace after tests when using helm (#7051) * Delete namespace after tests when using helm Signed-off-by: Neil Hickey <nhickey@pivotal.io> * Do not return error if namespace deletion fails If the helm chart being deleted has the namespace manifest then the HelmDelete would've already deleted the namespace * Add an e2e test for RedisQuota Adapter (#6720) * Add an e2e test for RedisQuota Adapter Fix RedisQuota Unit Test and lint error in kuberntes.go Remove additional log added in redisquota adapter delete the route rule after test finishes in mixer_test.go Fix vectorValue function and remove local port forwarding to redisquota server Add an e2e test for RedisQuota Adapter Fix RedisQuota Unit Test and lint error in kuberntes.go Remove additional log added in redisquota adapter delete the route rule after test finishes in mixer_test.go Add an e2e test for RedisQuota Adapter Fix RedisQuota Unit Test and lint error in kuberntes.go Remove additional log added in redisquota adapter delete the route rule after test finishes in mixer_test.go Fix vectorValue function and remove local port forwarding to redisquota server Add an e2e test for RedisQuota Adapter Fix RedisQuota Unit Test and lint error in kuberntes.go Remove additional log added in redisquota adapter delete the route rule after test finishes in mixer_test.go Remove destination version label for quota test Make calculations more tight for RedisQuota Format test Format test * Fix tests based on metrics change * Scale istio-policy pod to 2 for redisquota test * Fix spelling mistake in comment in kube_utils.go * Fix lint error with mixer_test.go * Fix lint error with mixer_test.go * Fix path for networking and policy rules * Move Kubernetes Ansible to proper location (#7207) * Fixing a wrong Json key for the CreationTimestamp field (#7206) * rename key for istio-remote chart repository->hub (#7209) * Move "repository" to "hub" and hardcode image name (#7187) * Move "repository" to "hub" and hardcode image name The imagename hard code comes from the pattern laid down in certmanager. I'm not super keen on this as it makes determining the image names for airgapped installs a bit more difficult. The good news is we can always add an "image" field for each service if we want to expose that information in values.yaml or just document it appropriately. * Move hyperkube back to globals * Fix a typo * Fix error in change * Sort configs by creation time before returning (#7216) * Sort configs by creation time before returning Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * format * Moved RBAC CRDs to group rbac.istio.io from config.istio.io (#6874) * Moved RBAC CRDs to group rbac.istio.io from config.istio.io * Review comments addressed * Modify global RbacConfig group too * Updated new files added in recent PRs * Updated a recently added RbacConfig CRD too * rbac.istio.io/v1alpha2 -> rbac.istio.io/v1alpha1 * Change protos version to v1alpha1 * Updated a test file that was recently added * Add useful warnings when quota specs do not match (#7226) * Add named logging scope for model * Add warning and tests to QuotaSpecByDestination 1. Add warning messages under correct conditions to inform user that some common error conditions were encountered. 2. Add test coverage for QuotaSpecByDestination. 3. Add previously omitted support for simple wildcard match. * review comments * update envoy sha - fix memory leak (#7232) Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * More debug/troubleshooting (#7227) * More debug/troubleshooting * Build error * Fix istio-remote sidecar-injector-configmap rendered yaml parse error (#7245) Fixes: #7244 * use receive only channel (#6960) * Tool to aggregate third party licenses (#7220) * Tool to aggregate third party licenses * Clean up and add match info as an option * Add readme, change binary name * Update readme, make --branch optional * Don't run licensee if not required * Lint * part 2 TCP routing cleanups/bug fixes for header matcher (#7236) * update envoy sha - fix memory leak Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * Fixing assorted bugs in service entries using filter chain matches Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * format * bug fix Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * lint Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * lint Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * multicluster bug fixes Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * backward compatibility Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * use new header match specifier Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * mixer: add sidecar health probe (#7102) * sidecar health Signed-off-by: Kuat Yessenov <kuat@google.com> * typos Signed-off-by: Kuat Yessenov <kuat@google.com> * remove tracing of health checks Signed-off-by: Kuat Yessenov <kuat@google.com> * change to 15093 Signed-off-by: Kuat Yessenov <kuat@google.com> * Add proxy version to the proxy-status command (#7269) * use new websocket option and remove deprecated one (#7247) * use new websocket option and remove deprecated one Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * backward compatibility Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * consistency Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * cleanup Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * dont append upgrade configs * remove requestedServerName attribute (#7278) it does not work, requires debugging and possibly implementation, will not be in time before the release * Handle virtual service sni_hosts matches in gateway. (#7192) * dirty poc * working poc * comments + cleanup * lint * add simple e2e test to egress * minor refactoring * cleanups, lots of comments Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * more duplication for clarity Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * simplifying gateway opaque tcp logic * final cleanups Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * nil pointer check Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com> * Add add operation into expression language (#6776) * add op. * fix duplicated test. * Mixer Conditional Quota (#7265) * add tests for conditional and unknown quota * add conditional and unknown quota * Option to disable pilot sidecar (#7280) * Remove whitespace between host: and port in 'istioctl authn tls-check' (#7084) * Remove whitespace between host: and port * Typo
Refactor the CRD group names of the RBAC related resources from
config.istio.io/v1alpha2torbac.istio.io/v1alpha1.Replaces #6820
cc @rshriram