Add API groups explanation to golang tutorial #5842
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jberkhahn
added
the
do-not-merge/hold
jberkhahn
removed
the
do-not-merge/hold
laxmikantbpandhare
suggested changes
Jun 9, 2022
|
/lgtm |
laxmikantbpandhare
approved these changes
Jul 25, 2022
|
/hold |
openshift-ci
bot
added
the
do-not-merge/hold
camilamacedo86
previously requested changes
Jul 27, 2022
There was a problem hiding this comment.
I think we need to shape this/improve a little this one. See: #5842 (comment)
/hold
jberkhahn
force-pushed
the
explain_group
branch
from
cc48528 to
4ec025c
Compare
Signed-off-by: Jonathan Berkhahn <[email protected]>
jberkhahn
force-pushed
the
explain_group
branch
from
4ec025c to
ac46d56
Compare
Comment on lines
40
to
44
| `--domain` will be used as the prefix of the API group your custom resources will be created in. | ||
| API groups are a mechanism to group portions of the Kubernetes API. You're probably already familiar with | ||
| some of the core Kubernetes API groups, such as `apps` or `rbac.authorization.k8s.io`. You should name your | ||
| domain to group your resource types in meaningful group(s), both for ease of understanding and because these | ||
| groups determine how access can be controlled to your resource types using RBAC. For more information, see [the core Kubernetes docs](https://kubernetes.io/docs/reference/using-api/#api-groups) and [the Kubebuilder docs](https://book.kubebuilder.io/cronjob-tutorial/gvks.html). |
There was a problem hiding this comment.
You should name your
domain to group your resource types in meaningful group(s), both for ease of understanding and because these
groups determine how access can be controlled to your resource types using RBAC.
@jberkhahn
I do not agree with the above text added in the PR. On top of that, after we all spoke about that, I think it should be a note and we might make it a little less verbose.
Suggested change
| `--domain` will be used as the prefix of the API group your custom resources will be created in. | |
| API groups are a mechanism to group portions of the Kubernetes API. You're probably already familiar with | |
| some of the core Kubernetes API groups, such as `apps` or `rbac.authorization.k8s.io`. You should name your | |
| domain to group your resource types in meaningful group(s), both for ease of understanding and because these | |
| groups determine how access can be controlled to your resource types using RBAC. For more information, see [the core Kubernetes docs](https://kubernetes.io/docs/reference/using-api/#api-groups) and [the Kubebuilder docs](https://book.kubebuilder.io/cronjob-tutorial/gvks.html). | |
| **Note:** By following the [Operators pattern](https://kubernetes.io/docs/concepts/extend-kubernetes/operator/) you will check in this tutorial that is possible to extend the Kubernetes API and create your own APIs. These APIs will be grouped by via the [ApiGroup](https://kubernetes.io/docs/reference/using-api/#api-groups). Then, the value informed via the `--domain` flag will be the subdomain of your API(s) (_i.e. the K8s ApiGroup `events.k8s.io` is the API group `events` grouped by `k8s.io` subdomain_). Then, this value should be meaningful to your organization and project. |
PS.: As we spoke in order we are able to track the detailed info related to this one ( mainly the motivations for this PR), the task #5979 was created.
Comment on lines
43
to
44
| domain to group your resource types in meaningful group(s), both for ease of understanding and because these | ||
| groups determine how access can be controlled to your resource types using RBAC. For more information, see [the core Kubernetes docs](https://kubernetes.io/docs/reference/using-api/#api-groups) and [the Kubebuilder docs](https://book.kubebuilder.io/cronjob-tutorial/gvks.html). |
There was a problem hiding this comment.
@jberkhahn IMO mentioning RBAC isn't necessary since that's not the only thing for which we have groups for.
Rest looks good, we can merge it.
Suggested change
| domain to group your resource types in meaningful group(s), both for ease of understanding and because these | |
| groups determine how access can be controlled to your resource types using RBAC. For more information, see [the core Kubernetes docs](https://kubernetes.io/docs/reference/using-api/#api-groups) and [the Kubebuilder docs](https://book.kubebuilder.io/cronjob-tutorial/gvks.html). | |
| domain to group your resource types in meaningful group(s). For more information, see [the core Kubernetes docs](https://kubernetes.io/docs/reference/using-api/#api-groups) and [the Kubebuilder docs](https://book.kubebuilder.io/cronjob-tutorial/gvks.html). |
laxmikantbpandhare
approved these changes
Sep 21, 2022
…golang tutorial Signed-off-by: Jonathan Berkhahn <[email protected]>
jmrodri
dismissed
camilamacedo86’s stale review
We all agreed we can merge this.
|
/hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #5738