Can't generate client and protobuf code simultaneously for API groups with dots in their name #25860

ericchiang · 2016-05-19T07:17:35Z

In #25634 I'm trying to add an API group called "rbac.authorization.k8s.io" which currently lives in the directory "pkg/apis/rbac"

Unfortunately, client generation with go2idl creates the file pkg/client/clientset_generated/internalclientset/typed/rbac/unversioned/rbac_client.go which has a snippet that looks like this:

func setConfigDefaults(config *restclient.Config) error {
    // if rbac group is not registered, return an error
    g, err := registered.Group("rbac")
    if err != nil {
        return err
    }
    ...
}

The string "rbac" is wrong since the group's name is "rbac.authorization.k8s.io". However changing it manually causes "hack/verify-codegen.sh" to fail.

errors in package "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/rbac/unversioned":
output for "unversioned/rbac_client.go" differs; first existing/expected diff: 
  ".authorization.k8s.io\")\n\tif err != nil {\n\t\treturn err\n\t}\n\tconfig.APIPath = \"/apis\"\n\tif config.UserAg"
  "\")\n\tif err != nil {\n\t\treturn err\n\t}\n\tconfig.APIPath = \"/apis\"\n\tif config.UserAgent == \"\" {\n\t\tconfig."

To deal with this I moved the directory from "pkg/apis/rbac" to "pkg/apis/rbac.authorization.k8s.io" so go2idl generates the correct string.

Now protobuf generation fails with a lot of error messages that look like this:

k8s.io/kubernetes/pkg/apis/rbac.authorization.k8s.io/v1alpha1/generated.proto:110:12: "k8s.io.kubernetes.pkg.api.v1.ObjectMeta" is resolved to "k8s.io.kubernetes.pkg.apis.rbac.authorization.k8s.io.kubernetes.pkg.api.v1.ObjectMeta", which is not defined. The innermost scope is searched first in name resolution. Consider using a leading '.'(i.e., ".k8s.io.kubernetes.pkg.api.v1.ObjectMeta") to start from the outermost scope.

From what I can tell, it's currently impossible to correctly generate assets for API groups which require client generation, if that group name has a dot in it.

Is there anything I can do to fix the protobuf or go2idl generation?

cc @erictune @sym3tri @gtank

The text was updated successfully, but these errors were encountered:

gtank · 2016-05-19T18:42:38Z

cc @caesarxuchao @smarterclayton for client-gen and protobuf namer, respectively

smarterclayton · 2016-05-19T20:37:35Z

Do you need dots in your package name?

ericchiang · 2016-05-19T20:45:16Z

@smarterclayton "rbac.authorization" or some to that affect was preferred per discussion in #23396, and #24900 but it's not an implementation requirement.

Would be happy to change the API group name if others are okay with that.

cc @erictune @liggitt @deads2k

edit: to be clearer I'd actually prefer this route so we can continue with #25634

smarterclayton · 2016-05-19T21:30:48Z

Can we just do "rbac"? Nothing in the code requires the package name to ==
the group name, so apis/rbac is totally clear already.

On Thu, May 19, 2016 at 5:05 PM, Eric Chiang notifications@github.com
wrote:

@smarterclayton https://github.com/smarterclayton "rbac.authorization"
or some to that affect was preferred per discussion in #23396
#23396, and #24900
#24900 but it's not an
implementation requirement.

Would be happy to change the API group name if others are okay with that.

cc @erictune https://github.com/erictune @liggitt
https://github.com/liggitt @deads2k https://github.com/deads2k

edit: to be clearer I'd actually prefer this route so we can continue with
#25634 #25634

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#25860 (comment)

ericchiang · 2016-05-19T21:52:48Z

Updating #25634 to rename the api group to "rbac". Will close this issue if there's no push back on that thread.

ericchiang · 2016-05-19T23:29:49Z

No push back. Closing for now.

deads2k · 2016-05-20T11:58:42Z

Do you need dots in your package name?

It nests under the existing authorization.k8s.io and matches the authentication.k8s.io, which also follows our plan to make the names dns names to avoid any ambiguity. We most recently talked about it here: #24902 (comment) .

deads2k · 2016-05-20T12:01:18Z

Can we just do "rbac"? Nothing in the code requires the package name to ==
the group name, so apis/rbac is totally clear already.

Ah, I see. Not renaming the API group, just the package.

smarterclayton · 2016-05-20T14:14:49Z

Sorry, should have been clearer.

liggitt · 2016-05-20T14:22:42Z

the group got renamed in https://github.com/kubernetes/kubernetes/pull/25634/files#diff-7e3f9c25e85c04b5583c2a61d4adf58eL25

I'd prefer the qualified group name, even if it means the package doesn't match

smarterclayton · 2016-05-20T16:46:25Z

Yes, please use the qualified group name, but the short package name.
Package names are for programmers, qualified group names are for api
consumers. Different needs.

Sorry for the confusion.

On Fri, May 20, 2016 at 10:23 AM, Jordan Liggitt notifications@github.com
wrote:

the group got renamed in
https://github.com/kubernetes/kubernetes/pull/25634/files#diff-7e3f9c25e85c04b5583c2a61d4adf58eL25

I'd prefer the qualified group name, even if it means the package doesn't
match

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#25860 (comment)

ericchiang · 2016-05-20T17:17:36Z

Sorry if I wasn't clear in my initial comment but the bug is, but the TL;DR is:

Client generation assumes that the directory name matches the API group name.
- Directory named "pkg/apis/rbac" with an API group named "rbac.authroization.k8s.io" doesn't work.
Protobuf generation does not work if the directory name has a dot in it.
- Directory named "pkg/apis/rbac.authorization.k8s.io" doesn't work.

The existing "authentication.k8s.io" group doesn't generate clients or protobufs, so it never saw this problem. The other API groups added during this release cycle, "app" and "policy" didn't either because they don't have a dot in their names.

ericchiang · 2016-05-20T17:18:08Z

So we can't use the qualified group name at the moment.

EDIT: I also don't have permission to re-open this issue.

smarterclayton · 2016-05-20T17:28:20Z

Ok, the first one is a definite bug. The second one was what I think is an
acceptable limitation.

Group names are APIs, so we can't change group name after creation, so if
we can't fix clientset generation we can't release this API. We need to
get the clientset thing sorted out.

On May 20, 2016, at 1:17 PM, Eric Chiang notifications@github.com wrote:

Sorry if I wasn't clear in my initial comment but the bug is, but the TL;DR
is:

Client generation assumes that the directory name matches the API
group name.
- Directory named "pkg/apis/rbac" with an API group named "
  rbac.authroization.k8s.io" doesn't work.
Protobuf generation does not work if the directory name has a dot in
it.
- Directory named "pkg/apis/rbac.authorization.k8s.io" doesn't work.

The existing "authentication.k8s.io" group doesn't generate clients or
protobufs, so it never saw this problem. The other API groups added during
this release cycle, "app" and "policy" didn't either because they don't
have a dot in their names.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#25860 (comment)

caesarxuchao · 2016-05-20T19:21:56Z

I don't want to demand a user to specify a const GroupName to be able to use client-gen. @smarterclayton is it so difficult to make protobuf generator to work with a directory having dots?

smarterclayton · 2016-05-20T19:28:52Z

Why not just read it from a comment on the package doc?

On May 20, 2016, at 3:22 PM, Chao Xu notifications@github.com wrote:

I don't want to demand a user to specify a const GroupName to be able to
use client-gen. @smarterclayton https://github.com/smarterclayton is it
so difficult to make protobuf generator to work with a directory having
dots?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#25860 (comment)

smarterclayton · 2016-05-20T19:29:07Z

Package names aren't special, we shouldn't assume they are.

On May 20, 2016, at 3:22 PM, Chao Xu notifications@github.com wrote:

I don't want to demand a user to specify a const GroupName to be able to
use client-gen. @smarterclayton https://github.com/smarterclayton is it
so difficult to make protobuf generator to work with a directory having
dots?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#25860 (comment)

caesarxuchao · 2016-05-20T20:15:09Z

If it's so hard to fix this from the protobuf side, I can let client-gen use the const groupName if it's present, or use the comment line as you suggested.

We should make a rule on how to name the directories under pkg/apis/, I vote for fully qualified name, so people don't need to search in the files to figure out the the group name.

smarterclayton · 2016-05-20T20:20:33Z

I'm more concerned that right now we're focused on enabling proto across
the board, so I don't want to dive on something optional if we can work
around it right now. Package names are conventions, and it sounds like
it's easy enough to support it in clientgen for now.

On Fri, May 20, 2016 at 4:15 PM, Chao Xu notifications@github.com wrote:

If it's so hard to fix this from the protobuf side, I can let client-gen
use the const groupName if it's present, or use the comment line as you
suggested.

We should make a rule on how to name the directories under pkg/apis/, I
vote for fully qualified name, so people don't need to search in the files
to figure out the the group name.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#25860 (comment)

ericchiang · 2016-05-27T17:17:01Z

This was fixed in #25634 by adding a logic to client generate which detect a doc.go comment. That comment is used at the actual group name instead of the directory name.

// +groupName={{ full group name }}

Directory names are still expected to not have a dot in them.

Example here:

kubernetes/pkg/apis/rbac/doc.go

Line 17 in 294e49f

// +groupName=rbac.authorization.k8s.io

smarterclayton · 2016-06-12T22:44:52Z

Protobuf has pretty strict rules on what sorts of names can be in package definitions (basically [a-z0-9_]. We bypass this for k8s.io because our search path starts at k8s.io/kubernetes, but for nested paths we can't. If we can't use the package name directly, we have to either put all the .protos in one package (possible) or we have to use only valid package names for folders.

If we put all the protos in one place, it'll probably be something like apis/protos. I'll look into that.

yue9944882 · 2019-09-10T16:15:07Z

We bypass this for k8s.io because our search path starts at k8s.io/kubernetes, but for nested paths we can't.

am probably hitting this issue 3 three years later from the discussion.. i was trying to generate protobuf codes from a repo under github.com/kubernetes-sigs i.e. sigs.k8s.io. but it failed w/ error shown below:

2019/09/10 23:57:06 sigs.k8s.io/apiserver-builder-alpha/example/basic/pkg/apis/innsmouth/v1/generated.proto:43:12: "k8s.io.apimachinery.pkg.apis.meta.v1.ObjectMeta" is resolved to "sigs.k8s.io.apimachinery.pkg.apis.meta.v1.ObjectMeta", which is not defined. The innermost scope is searched first in name resolution. Consider using a leading '.'(i.e., ".k8s.io.apimachinery.pkg.apis.meta.v1.ObjectMeta") to start from the outermost scope.
sigs.k8s.io/apiserver-builder-alpha/example/basic/pkg/apis/innsmouth/v1/generated.proto:51:12: "k8s.io.apimachinery.pkg.apis.meta.v1.ListMeta" is resolved to "sigs.k8s.io.apimachinery.pkg.apis.meta.v1.ListMeta", which is not defined. The innermost scope is searched first in name resolution. Consider using a leading '.'(i.e., ".k8s.io.apimachinery.pkg.apis.meta.v1.ListMeta") to start from the outermost scope.

from the error, i think the generator is somehow mixing up sigs.k8s.io w/ k8s.io. when i copied the project from $GOPATH/sigs.k8s.io to $GOPATH/k8s.io using simply cp -r , the protobuf generator began to work perfectly.. how can i make it work?

lavalamp added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. labels May 19, 2016

ericchiang mentioned this issue May 19, 2016

Add RBAC authorization API group and authorizer #25634

Merged

ericchiang closed this as completed May 19, 2016

deads2k reopened this May 20, 2016

deads2k closed this as completed May 20, 2016

liggitt reopened this May 20, 2016

caesarxuchao mentioned this issue May 20, 2016

[client-gen] allow overriding group name in URL via comments in the doc of the group's package #25996

Closed

ericchiang closed this as completed May 27, 2016

smarterclayton self-assigned this Jun 12, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Can't generate client and protobuf code simultaneously for API groups with dots in their name #25860

Can't generate client and protobuf code simultaneously for API groups with dots in their name #25860

ericchiang commented May 19, 2016 •

edited

Loading

gtank commented May 19, 2016

smarterclayton commented May 19, 2016

ericchiang commented May 19, 2016 •

edited

Loading

smarterclayton commented May 19, 2016

ericchiang commented May 19, 2016

ericchiang commented May 19, 2016

deads2k commented May 20, 2016

deads2k commented May 20, 2016

smarterclayton commented May 20, 2016 via email

liggitt commented May 20, 2016

smarterclayton commented May 20, 2016

ericchiang commented May 20, 2016

ericchiang commented May 20, 2016 •

edited

Loading

smarterclayton commented May 20, 2016

caesarxuchao commented May 20, 2016

smarterclayton commented May 20, 2016

smarterclayton commented May 20, 2016

caesarxuchao commented May 20, 2016

smarterclayton commented May 20, 2016

ericchiang commented May 27, 2016

smarterclayton commented Jun 12, 2016

yue9944882 commented Sep 10, 2019 •

edited

Loading

Can't generate client and protobuf code simultaneously for API groups with dots in their name #25860

Can't generate client and protobuf code simultaneously for API groups with dots in their name #25860

Comments

ericchiang commented May 19, 2016 • edited Loading

gtank commented May 19, 2016

smarterclayton commented May 19, 2016

ericchiang commented May 19, 2016 • edited Loading

smarterclayton commented May 19, 2016

ericchiang commented May 19, 2016

ericchiang commented May 19, 2016

deads2k commented May 20, 2016

deads2k commented May 20, 2016

smarterclayton commented May 20, 2016 via email

liggitt commented May 20, 2016

smarterclayton commented May 20, 2016

ericchiang commented May 20, 2016

ericchiang commented May 20, 2016 • edited Loading

smarterclayton commented May 20, 2016

caesarxuchao commented May 20, 2016

smarterclayton commented May 20, 2016

smarterclayton commented May 20, 2016

caesarxuchao commented May 20, 2016

smarterclayton commented May 20, 2016

ericchiang commented May 27, 2016

smarterclayton commented Jun 12, 2016

yue9944882 commented Sep 10, 2019 • edited Loading

ericchiang commented May 19, 2016 •

edited

Loading

ericchiang commented May 19, 2016 •

edited

Loading

ericchiang commented May 20, 2016 •

edited

Loading

yue9944882 commented Sep 10, 2019 •

edited

Loading