Role, RoleBinding, ClusterRole, ClusterRoleBinding handlers are registered with wrong key

[leogps]

kubernetes-client/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/handlers/KubernetesRoleHandler.java

Line 38 in 0f30508

return KubernetesRole.class.getSimpleName();

It should be "Role" not "KubernetesRole". Using Class.getSimpleName() returns the class name.

Similar issue in the following classes:

• KubernetesRoleBinding

--> RoleBinding; Affected class KubernetesRoleBindingHandler.

• KubernetesClusterRole

--> ClusterRole; Affected class KubernetesClusterRoleHandler.

• KubernetesClusterRoleBinding

--> ClusterRoleBinding; Affected class KubernetesClusterRoleBindingHandler.

[fnan-avq]

Is there any plan to fix this in a coming release ?

[rohanKanojia]

Yeah, we'll fix this in coming release

[traviswinter]

@rohanKanojia - do you know if there was a reason Kubernetes was added to the front of those class names? Just to avoid collision with the Openshift resources? Would this be as simple as renaming them in the kubeschema.json?

[rohanKanojia]

ah, no. I need to check

[rohanKanojia]

@traviswinter :You're right Kubernetes was added in order to avoid collision with Openshift Role Bindings(Since they were implemented before); I think we need to remove prefixes from here to rename it:

kubernetes-client/kubernetes-model/cmd/generate/generate.go

Lines 196 to 200 in 99cae30

	K8sRole rbac.Role
	K8sRoleList rbac.RoleList
	K8sRoleBinding rbac.RoleBinding
	K8sRoleBindingList rbac.RoleBindingList
	NetNameSpace networkapi.NetNamespace

But I'm afraid doing this might break backward compatibility for openshift users.

[batsatt]

Can you just change the getKind() implementations to return the correct string constant instead of the (simple) class name?

We are blocked on this bug.

[batsatt]

I've worked around this for now by making renamed copies of the classes and manually registering them. (And no, modifying getKind() was not enough.)

[traviswinter]

@rohanKanojia - What do you think we should do here? Any idea on how we can prevent breaking of Openshift backwards compatibility? I was looking into just checking for "Kubernetes"+getKind() in Handler but that could potentially break the OpenShift handlers as well.

Going to try and get more familiar with this code (now that I found out how to add the Eclipse key-bindings to IntelliJ :)) so I can be more productive.

[rohanKanojia]

@traviswinter : This is creating lots of confusion among users. I think we should move forward with renaming these classes to their proper names and rename Openshift classes with prefixes( I think this would make more sense).

@iocanel @oscerd : thoughts???

[oscerd]

+1, makes sense.

[rohanKanojia]

@traviswinter : Would be awesome if you can find time to fix this. You just need to rename classes in generate.go and resolve some conflicts in client code due to newly generated model. Otherwise, I would try to send PR this weekend :-)

[traviswinter]

@rohanKanojia , sure - can take a stab at it this afternoon. Any recommendation for the prefixes? Simply call it "Openshift"+kind? Would this simply trade the issue from Kubernetes to Openshift? Or does Openshift require a different pattern for creating their resources?

[rohanKanojia]

@traviswinter : No, it's the same pattern. It's just that for an Openshift object we read from an openshift vendor directory(in case of RoleBinding github.com/openshift/api/authorization/v1). If I'm not wrong these Role bindings were introduced earlier in Openshift and then came into kubernetes(the generated classes also look similar). Now kubernetes also supports them so as an openshift user I would prefer to use kubernetes based RoleBindings(that's just my opinion ;-) ).

[traviswinter]

Looks like the generate.go changes were not enough - changes to kube-schema.json were required as well, or am I doing something wrong? :)

The generated classes themselves will need to be changed as well for
return KubernetesRole.class.getSimpleName(); to work in KubernetesRoleHandler.java.

[singhania]

In which version this will be released ?

[rohanKanojia]

@singhania : It would be available in next release i.e 4.1.2

[PMarci]

Could this be the cause of my Kubernetes RoleBinding fragment not being picked up by the fabric8-maven-plugin? It's located in the same (default) directory as other fragments, which are created successfully. I can also see from the logs and by inspecting the generated OpenShift manifest that the equivalent OpenShift RoleBinding is in fact generated.