Namespace certificates API group #31887

liggitt · 2016-09-01T15:42:24Z

New API groups should follow best-practices for naming, including using DNS names within the k8s.io namespace

The certificates API group has been renamed to certificates.k8s.io

This change is

k8s-bot · 2016-09-01T15:45:01Z

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message will repeat several times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

k8s-bot · 2016-09-01T15:45:25Z

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message will repeat several times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

liggitt · 2016-09-01T15:48:23Z

@k8s-bot ok to test

mikedanese · 2016-09-01T16:59:35Z

I'd like to get this in for v1.4

On Thu, Sep 1, 2016 at 8:49 AM, Jordan Liggitt [email protected]
wrote:

Assigned #31887 #31887 to
@mikedanese https://github.com/mikedanese.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#31887 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABtFIU782XkhLrEuOSgQdRWUCnftbJPZks5qlvQCgaJpZM4Jy1kC
.

mikedanese · 2016-09-01T18:44:42Z

@pwittrock requesting that this be included in v1.4 milestone.

How risky is it to merge this change?

Not risky. This is a rest path change of an API that will be introduced as alpha in v1.4.

How easy is it to roll this change back?

Not hard. This is mostly a rename. Some autogenerated swagger files are touched but the change is isolated to certificates API sections of those files so there won't be merge conflicts.

What's the cost if it doesn't make the 1.4 release?

Even if we wanted to make a compatible new API version of this API, we could not. Although the API is alpha and we reserve the right to break it, we should aim to avoid this if at all possible. Without this change, all API consumers will break on upgrade from v1.4 to v1.5.

cc @gtank

k8s-bot · 2016-09-01T19:18:09Z

GCE e2e build/test passed for commit a869de6.

liggitt · 2016-09-01T19:31:56Z

tests are green, all the verify scripts are happy

brought up an API server and tested with:

$ kubectl create -f - <<< '{"kind":"CertificateSigningRequest","apiVersion":"certificates.k8s.io/v1alpha1","metadata":{"name":"foo"},"spec":{"request":"LS0tLS1CRUdJTiBDRVJUSUZJQ0FURSBSRVFVRVNULS0tLS0KTUlJQ3ZEQ0NBYVFDQVFBd2R6RUxNQWtHQTFVRUJoTUNWVk14RFRBTEJnTlZCQWdNQkZWMFlXZ3hEekFOQmdOVgpCQWNNQmt4cGJtUnZiakVXTUJRR0ExVUVDZ3dOUkdsbmFVTmxjblFnU1c1akxqRVJNQThHQTFVRUN3d0lSR2xuCmFVTmxjblF4SFRBYkJnTlZCQU1NRkdWNFlXMXdiR1V1WkdsbmFXTmxjblF1WTI5dE1JSUJJakFOQmdrcWhraUcKOXcwQkFRRUZBQU9DQVE4QU1JSUJDZ0tDQVFFQTgrVG83ZCsya1BXZUJ2L29yVTNMVmJKd0RyU1FiZUthbUNtbwp3cDVicUR4SXdWMjB6cVJiN0FQVU9LWW9WRUZGT0VRczZUNmdJbW5Jb2xoYmlINm00emdaL0NQdldCT2taYytjCjFQbzJFbXZCeitBRDVzQmRUNWt6R1FBNk5iV3laR2xkeFJ0aE5MT3MxZWZPaGRuV0Z1aEkxNjJxbWNmbGdwaUkKV0R1d3E0QzlmK1lrZUpoTm45ZEY1K293bThjT1FtRHJWOE5OZGlUcWluOHEzcVlBSEhKUlcyOGdsSlVDWmtUWgp3SWFTUjZjckJROFRiWU5FMGRjK0NhYTNET0lrejFFT3NIV3pUeCtuMHpLZnFjYmdYaTRESngrQzFianB0WVBSCkJQWkw4REFlV3VBOGVidWRWVDQ0eUVwODJHOTYvR2djZjdGMzN4TXhlMHljK1hhNm93SURBUUFCb0FBd0RRWUoKS29aSWh2Y05BUUVGQlFBRGdnRUJBQjBrY3JGY2NTbUZEbXhveDBOZTAxVUlxU3NEcUhnTCtYbUhUWEp3cmU2RApoSlNad2J2RXRPSzBHMytkcjRGczExV3VVTnQ1cWNMc3g1YTh1azRHNkFLSE16dWhMc0o3WFpqZ21RWEdFQ3BZClE0bUMzeVQzWm9DR3BJWGJ3K2lQM2xtRUVYZ2FRTDBUeDVMRmwvb2tLYktZd0lxTml5S1dPTWo3WlIvd3hXZy8KWkRHUnM1NXh1b2VMREovWlJGZjliSStJYUNVZDFZcmZZY0hJbDNHODdBdityNDlZVndxUkRUMFZEVjd1TGdxbgoyOVhJMVBwVlVOQ1BRR245cC9lWDZRbzd2cERhUHliUnRBMlI3WExLalFhRjlvWFdlQ1VxeTFodkphYzlRRk8yCjk3T2IxYWxwSFBvWjdtV2lFdUp3akJQaWk2YTlNOUczMG5VbzM5bEJpMXc9Ci0tLS0tRU5EIENFUlRJRklDQVRFIFJFUVVFU1QtLS0tLQo="}}'

certificatesigningrequest "foo" created

$ kubectl describe csr
Name:           foo
Labels:         <none>
Annotations:        <none>
CreationTimestamp:  Thu, 01 Sep 2016 15:30:21 -0400
Requesting User:    
Status:         Pending
Subject:
    Common Name:        example.digicert.com
    Serial Number:      
    Organization:       DigiCert Inc.
    Organizational Unit:    DigiCert
    Country:        US
    Locality:       Lindon
    Province:       Utah
No events.

verified requests are hitting the /apis/certificates.k8s.io endpoint

mikedanese · 2016-09-01T19:39:45Z

LGTM, @caesarxuchao can you make sure the api bits look fine?

caesarxuchao · 2016-09-01T20:14:29Z

I thought the decision in the sig meeting yesterday is using "kubernetes.io", not "k8s.io"? @lavalamp

liggitt · 2016-09-01T21:32:35Z

I thought the decision in the sig meeting yesterday is using "kubernetes.io", not "k8s.io"?

I didn't think there was a strong preference, but maybe I misread. For consistency with our import package (k8s.io), existing API groups (see below), and ease of use, I went with the shorter name.

existing API groups:

$ find . -name register.go | xargs -n 1 grep 'GroupName = ' | sort -u
const GroupName = ""
const GroupName = "apps"
const GroupName = "authentication.k8s.io"
const GroupName = "authorization.k8s.io"
const GroupName = "autoscaling"
const GroupName = "batch"
const GroupName = "certificates.k8s.io"
const GroupName = "componentconfig"
const GroupName = "extensions"
const GroupName = "federation"
const GroupName = "imagepolicy.k8s.io"
const GroupName = "policy"
const GroupName = "rbac.authorization.k8s.io"

lavalamp · 2016-09-01T21:42:30Z

@bgrant0607 prefers it spelled out. Not sure if that list will convince him otherwise but we can ask?

bgrant0607 · 2016-09-02T04:28:54Z

Sadly, we're not consistent. It depends on who does the API review.

We have API groups qualified with domains and those without.

We use .k8s.io in the cases above and .kubernetes.io in most annotations.

My general position for the API is to not abbreviate.
https://github.com/kubernetes/kubernetes/blob/master/docs/devel/api-conventions.md#naming-conventions

Kubernetes contributors know what K8s means, but I wouldn't expect users to. However, I don't hear people being confused by K8s as often as I used to.

In any case, my other rule is that consistency trumps correctness, so if we've used .k8s.io in other API group names but not .kubernetes.io, then stick with .k8s.io.

cc @thockin @erictune

thockin · 2016-09-02T05:37:33Z

k8s.io is supposed to be shorthand. I've intentionally used kubernetes.io for all public domains. The one exception is the go-get path, and perhaps that was a mistake.

That said, it IS shorter and I am lazy. We should use the short form in places that humans type a lot. I don't think this is one of those cases. Inconsistency annoys me, so if the ship has sailed, then consistency wins. If it's not too late, I'd still argue for "kubernetes.io".

I suppose aliasing it with "secondary" names is right out? Accept either but always emit kubernetes.io .. ?

gtank · 2016-09-02T17:40:57Z

Good change! I would also like to see this in 1.4. Consistency with the other API groups (and future norms) should be done before anyone depends on this naming scheme.

It may be a developer-centric opinion but, as someone who has typed these names frequently, I side with using .k8s.io.

mikedanese · 2016-09-02T17:45:47Z

We need to make a decision on this. Going with kubernetes.io will require migrating 4 other api groups and we should do that before v1.4. I am not opinionated.

caesarxuchao · 2016-09-02T17:50:26Z

staging/src/k8s.io/client-go/1.4/pkg/apis/certificates/doc.go

@@ -14,6 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */

+// +groupName=certificates.k8s.io


How did you change the staging area? It looks like you manually changed it?

Anyway, the changes look fine.

yes, was manual, found it in a search for the old name

liggitt · 2016-09-02T18:55:30Z

@bgrant0607
In any case, my other rule is that consistency trumps correctness, so if we've used .k8s.io in other API group names but not .kubernetes.io, then stick with .k8s.io.

@thockin
Inconsistency annoys me, so if the ship has sailed, then consistency wins. If it's not too late, I'd still argue for "kubernetes.io".

@mikedanese
Going with kubernetes.io will require migrating 4 other api groups and we should do that before v1.4.

Agree on being consistent. Since some of the suffixed groups existed in 1.3, switching to kubernetes.io would be pretty disruptive, not just for the 1.4 release. Without objection, I'll leave the PR as-is (and open a pick to the release-1.4 branch)

I suppose aliasing it with "secondary" names is right out? Accept either but always emit kubernetes.io .. ?

we could look into aliasing groups in future releases, but in the meantime, keeping things consistent seems ideal

bgrant0607 · 2016-09-02T19:08:03Z

I agree with .k8s.io.

Please no aliasing. It's confusing for users to see the multiple different forms in use and returning a different form than what the client expected won't work -- imagine accepting multiple apiversions but returning only 1.

k8s-github-robot · 2016-09-02T21:32:14Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

k8s-bot · 2016-09-02T22:07:28Z

GCE e2e build/test passed for commit a869de6.

k8s-github-robot · 2016-09-02T22:08:55Z

Automatic merge from submit-queue

thockin · 2016-09-05T05:12:59Z

we really need to get in from of the k8s.io vs kubernetes.io mess and lay
some rules, even if they are arbitrary.

On Fri, Sep 2, 2016 at 3:09 PM, Kubernetes Submit Queue <
[email protected]> wrote:

Merged #31887 #31887.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#31887 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AFVgVAR56NKAyOzzNuuZjg13pg-givDUks5qmJ6ugaJpZM4Jy1kC
.

Automatic merge from submit-queue Doc API group suffix, add test to catch new groups Spawned from discussion in #31887 Doc and add tests to ensure new API groups are suffixed. Also changed the doc to reference an API group containing the suffix as a starting point for new API groups.

Automatic merge from submit-queue Doc API group suffix, add test to catch new groups Spawned from discussion in kubernetes/kubernetes#31887 Doc and add tests to ensure new API groups are suffixed. Also changed the doc to reference an API group containing the suffix as a starting point for new API groups.

googlebot added the cla: yes label Sep 1, 2016

k8s-github-robot added kind/new-api size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 1, 2016

liggitt assigned mikedanese Sep 1, 2016

liggitt force-pushed the certificates-group branch from 3829555 to 47d1fd0 Compare September 1, 2016 15:50

liggitt force-pushed the certificates-group branch 3 times, most recently from a89b830 to 07d4ae7 Compare September 1, 2016 17:41

liggitt added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed labels Sep 1, 2016

k8s-github-robot mentioned this pull request Sep 1, 2016

[k8s.io] ScheduledJob should replace jobs when ReplaceConcurrent {Kubernetes e2e suite} #31552

Closed

Namespace certificates API group

a869de6

liggitt force-pushed the certificates-group branch from 07d4ae7 to a869de6 Compare September 1, 2016 18:39

k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 1, 2016

liggitt added this to the v1.4 milestone Sep 1, 2016

mikedanese assigned caesarxuchao Sep 1, 2016

caesarxuchao reviewed Sep 2, 2016
View reviewed changes

mikedanese added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 2, 2016

liggitt mentioned this pull request Sep 2, 2016

Automated cherry pick of #31887 #31997

Closed

k8s-github-robot merged commit 11765ab into kubernetes:master Sep 2, 2016

k8s-github-robot mentioned this pull request Sep 2, 2016

[k8s.io] Kubectl client [k8s.io] Kubectl taint should remove all the taints with the same key off a node {Kubernetes e2e suite} #31967

Closed

liggitt deleted the certificates-group branch September 8, 2016 14:58

liggitt mentioned this pull request Sep 9, 2016

Doc API group suffix, add test to catch new groups #32315

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Namespace certificates API group #31887

Namespace certificates API group #31887

liggitt commented Sep 1, 2016 •

edited

Loading

k8s-bot commented Sep 1, 2016

k8s-bot commented Sep 1, 2016

liggitt commented Sep 1, 2016

mikedanese commented Sep 1, 2016

mikedanese commented Sep 1, 2016 •

edited

Loading

k8s-bot commented Sep 1, 2016

liggitt commented Sep 1, 2016

mikedanese commented Sep 1, 2016

caesarxuchao commented Sep 1, 2016

liggitt commented Sep 1, 2016

lavalamp commented Sep 1, 2016

bgrant0607 commented Sep 2, 2016

thockin commented Sep 2, 2016

gtank commented Sep 2, 2016 •

edited

Loading

mikedanese commented Sep 2, 2016

caesarxuchao Sep 2, 2016

caesarxuchao Sep 2, 2016

liggitt Sep 2, 2016

liggitt commented Sep 2, 2016

bgrant0607 commented Sep 2, 2016

k8s-github-robot commented Sep 2, 2016

k8s-bot commented Sep 2, 2016

k8s-github-robot commented Sep 2, 2016

thockin commented Sep 5, 2016

Namespace certificates API group #31887

Namespace certificates API group #31887

Conversation

liggitt commented Sep 1, 2016 • edited Loading

k8s-bot commented Sep 1, 2016

k8s-bot commented Sep 1, 2016

liggitt commented Sep 1, 2016

mikedanese commented Sep 1, 2016

mikedanese commented Sep 1, 2016 • edited Loading

k8s-bot commented Sep 1, 2016

liggitt commented Sep 1, 2016

mikedanese commented Sep 1, 2016

caesarxuchao commented Sep 1, 2016

liggitt commented Sep 1, 2016

lavalamp commented Sep 1, 2016

bgrant0607 commented Sep 2, 2016

thockin commented Sep 2, 2016

gtank commented Sep 2, 2016 • edited Loading

mikedanese commented Sep 2, 2016

caesarxuchao Sep 2, 2016

Choose a reason for hiding this comment

caesarxuchao Sep 2, 2016

Choose a reason for hiding this comment

liggitt Sep 2, 2016

Choose a reason for hiding this comment

liggitt commented Sep 2, 2016

bgrant0607 commented Sep 2, 2016

k8s-github-robot commented Sep 2, 2016

k8s-bot commented Sep 2, 2016

k8s-github-robot commented Sep 2, 2016

thockin commented Sep 5, 2016

liggitt commented Sep 1, 2016 •

edited

Loading

mikedanese commented Sep 1, 2016 •

edited

Loading

gtank commented Sep 2, 2016 •

edited

Loading