Skip to content

[WIP] Refactor RBAC crds's group name#6820

Closed
ymesika wants to merge 8 commits intoistio:masterfrom
ymesika:rbacRefactor
Closed

[WIP] Refactor RBAC crds's group name#6820
ymesika wants to merge 8 commits intoistio:masterfrom
ymesika:rbacRefactor

Conversation

@ymesika
Copy link
Copy Markdown
Member

@ymesika ymesika commented Jul 3, 2018

Refactor the CRD group names of the RBAC related resources from config.istio.io/v1alpha2 to rbac.istio.io/v1alpha2.

cc @rshriram

@istio-testing istio-testing added the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Jul 3, 2018
@istio-testing
Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ymesika
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: costinm

Assign the PR to them by writing /assign @costinm in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@codecov
Copy link
Copy Markdown

codecov bot commented Jul 3, 2018

Codecov Report

Merging #6820 into master will decrease coverage by 1%.
The diff coverage is 62%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #6820    +/-   ##
=======================================
- Coverage      71%     71%   -<1%     
=======================================
  Files         369     369            
  Lines       31907   31729   -178     
=======================================
- Hits        22420   22231   -189     
- Misses       8574    8577     +3     
- Partials      913     921     +8
Impacted Files Coverage Δ
pilot/pkg/model/config.go 49% <ø> (ø) ⬆️
mixer/pkg/config/crd/init.go 0% <0%> (ø) ⬆️
mixer/adapter/rbac/rbac.go 11% <0%> (ø) ⬇️
galley/pkg/kube/types.go 100% <100%> (ø) ⬆️
mixer/pkg/config/store/store.go 89% <100%> (ø) ⬆️
mixer/pkg/server/server.go 95% <100%> (+1%) ⬆️
mixer/pkg/config/crd/store.go 99% <100%> (ø) ⬆️
...olarwinds/internal/papertrail/papertrail_logger.go 53% <0%> (-9%) ⬇️
galley/pkg/mcp/client/client.go 71% <0%> (-6%) ⬇️
mixer/adapter/cloudwatch/handler.go 87% <0%> (-4%) ⬇️
... and 24 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 865e90e...b880ea3. Read the comment docs.

@ymesika ymesika changed the base branch from master to release-1.0 July 5, 2018 18:52
@ymesika ymesika changed the base branch from release-1.0 to master July 5, 2018 18:52
@istio-testing
Copy link
Copy Markdown
Collaborator

istio-testing commented Jul 5, 2018

@ymesika: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
prow/istio-unit-tests.sh b880ea3 link /test istio-unit-tests
Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

apiVersion = "v1alpha2"
apiGroupVersion = apiGroup + "/" + apiVersion
)

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.

@douglas-reid / @geeknoid et al was there any reason to have this in a generic config.istio.io instead of .istio.io ?

@ymesika
Copy link
Copy Markdown
Member Author

ymesika commented Jul 6, 2018

Replaced by #6874 with release-1.0 as the base branch.

@ozevren
Copy link
Copy Markdown
Contributor

ozevren commented Jul 6, 2018

An approach like this can help support old and new API Groups side-by-side:
https://github.com/ozevren/istio/commit/9ce843eb63c32757c079a62d8c4b2637e6520622

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.

@ymesika ymesika closed this Jul 7, 2018
@ymesika ymesika deleted the rbacRefactor branch July 14, 2018 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants