Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #1152: Renamed Kubernetes RBAC resources to use non-prefixed names and renamed Openshift RBAC resources to prefix with Openshift #1300

Merged
merged 3 commits into from
Dec 24, 2018

Conversation

traviswinter
Copy link
Contributor

@traviswinter traviswinter commented Dec 21, 2018

Not sure if this level of change is what you meant @rohanKanojia but I think it cleans the Kubernetes RBAC references up quite a bit. It does change the client api (eg. rbac().kubernetesClusterRoles() becomes rbac().clusterRoles()) but I believe it would be a little more intuitive.

If those changes are too severe, I can back off the renaming of the methods at least.

Key changes are in generate.go, kube-schema.json and validation-schema.json

Fixes #1152

…s and renamed Openshift RBAC resources to prefix with Openshift
@centos-ci
Copy link

Can one of the admins verify this patch?

@rohanKanojia
Copy link
Member

ok to test

ClusterRole authapi.ClusterRole
ClusterRoleBinding authapi.ClusterRoleBinding
ClusterRoleBindingList authapi.ClusterRoleBindingList
SubjectAccessReviewResponse authapi.SubjectAccessReviewResponse
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we also prefix above RoleBinding classes from authapi with Openshift also?

Role            authapi.Role        -> OpenshiftRole                authapi.Role
RoleList      authapi.RoleList   -> OpenshiftRoleList          authapi.RoleList
....

@rohanKanojia
Copy link
Member

Also run mvn -N license:format in order to fix license headers :-)

Copy link
Member

@rohanKanojia rohanKanojia left a comment

Choose a reason for hiding this comment

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

Minor comments otherwise looks good, Thanks 👍

@traviswinter
Copy link
Contributor Author

Oh, good catch with the OpenshiftRole and OpenshiftRoleList! I'll do that and run the license formatter when I get a chance in a couple of hours :)

@traviswinter
Copy link
Contributor Author

Looks like an infrastructure failure? Is there a way to kick it off again?

Deleting project: itest-afbbbec6...
Project: itest-afbbbec6, successfully deleted
Destroying Session:afbbbec6
[INFO]
[INFO] Results:
[INFO]
[ERROR] Errors:
[ERROR] DeploymentIT.delete:131 » ConditionTimeout Condition io.fabric8.commons.ReadyE...
[ERROR] DeploymentIT.update:121 » ConditionTimeout Condition io.fabric8.commons.ReadyE...
[ERROR] DeploymentIT.waitTest:139 » ConditionTimeout Condition io.fabric8.commons.Read...
[ERROR] PodIT.exec:129 » ConditionTimeout Condition io.fabric8.commons.ReadyEntity was...
[ERROR] PodIT.log:120 » ConditionTimeout Condition io.fabric8.commons.ReadyEntity was ...
[INFO]
[ERROR] Tests run: 130, Failures: 0, Errors: 5, Skipped: 0

@rohanKanojia
Copy link
Member

@traviswinter : Yeah, I restarted job.

@rohanKanojia
Copy link
Member

[merge]

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.

5 participants