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

Resource name constraints (1) #19106

Merged
merged 1 commit into from
Feb 19, 2020
Merged

Conversation

tengqm
Copy link
Contributor

@tengqm tengqm commented Feb 13, 2020

xref: #17969, #19099, #18746

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Feb 13, 2020
@netlify
Copy link

netlify bot commented Feb 13, 2020

Deploy preview for kubernetes-io-master-staging ready!

Built with commit e7d6a34

https://deploy-preview-19106--kubernetes-io-master-staging.netlify.com

@tengqm tengqm force-pushed the name-constraints branch 2 times, most recently from 9032a91 to 82d6723 Compare February 14, 2020 03:00
@kbhawkey
Copy link
Contributor

@tengqm , I am reading through the updates. I just noticed that the title
of the page with the changes is called Names and the main h2 heading is also called
Names. What do you think about updating the title or updating the first heading?

https://deploy-preview-19106--kubernetes-io-master-staging.netlify.com/docs/concepts/overview/working-with-objects/names/#names

@@ -42,6 +69,7 @@ spec:
- containerPort: 80
```


Copy link
Contributor

Choose a reason for hiding this comment

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

Another nit:
Is the Pod manifest a good example of naming conventions or could you provide some different examples of
Kubernetes resource names.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pod is the basic building block in Kubernetes, so I think keeping it here makes sense.
We can certainly provide more examples here or somewhere else but I don't see it necessary.
In follow-up PRs, there are references to these resource name constraints.

@tengqm
Copy link
Contributor Author

tengqm commented Feb 16, 2020

the title of the page with the changes is called Names and the main h2 heading is also called
Names.

Em... that is not ideal. Changing either one may break the links or bookmarks people have. For example, if we change either one, the existing references to /docs/concepts/overview/working-with-objects/names or /docs/concepts/overview/working-with-objects/names#names will break. So I don't see it necessary. Maybe we can revisit this when we have a checker for dangling links.

Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

@tengqm - how about referring to https://tools.ietf.org/html/rfc2181#section-11 for the definition of DNS labels and names?

@sftim
Copy link
Contributor

sftim commented Feb 16, 2020

I think it's OK to risk breaking https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names as it's the first content heading. If it's important to preserve it, it's feasible to add an element to the page at pretty much the right place (eg just before the renamed heading), and set its id attribute to names.

@sftim
Copy link
Contributor

sftim commented Feb 16, 2020

We could rename the page to “Object Names and UIDs", then add a redirect to its new URL.

### DNS Label Names

There are some resource names are required to conform to the DNS
label standard as defined in [RFC 1123](https://tools.ietf.org/html/rfc1123).
Copy link
Contributor

Choose a reason for hiding this comment

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

@tengqm , changes look good. One nit, reword line 42. What do you think about these changes:
Some resources require their names to conform to the DNS label standard ...
Some resource types require their names to follow the DNS label standard ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@kbhawkey
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 19, 2020
Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

Looks good to me, too!
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sftim

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

The pull request process is described here

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 19, 2020
@k8s-ci-robot k8s-ci-robot merged commit ceccbc0 into kubernetes:master Feb 19, 2020
zacharysarah pushed a commit to zacharysarah/website that referenced this pull request Feb 21, 2020
wawa0210 pushed a commit to wawa0210/website that referenced this pull request Mar 2, 2020
@tengqm tengqm deleted the name-constraints branch March 6, 2020 03:51
VineethReddy02 pushed a commit to VineethReddy02/website that referenced this pull request Mar 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants