-
Notifications
You must be signed in to change notification settings - Fork 362
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
Add IP based group association query API and ClientSet #4807
Conversation
273da07
to
3c745d3
Compare
6ba6d6f
to
9484a92
Compare
@@ -393,23 +402,34 @@ func (c *NetworkPolicyController) GetAssociatedGroups(name, namespace string) ([ | |||
|
|||
// getAssociatedGroupsByName retrieves the internal Group and all it's parent Group objects | |||
// (if any) by Group name. | |||
func (c *NetworkPolicyController) getAssociatedGroupsByName(grpName string) []antreatypes.Group { | |||
func (c *NetworkPolicyController) getAssociatedGroupsByName(grpName string) ([]antreatypes.Group, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should clean up such impossible error, instead of adding more. Accessing in-memory indexer can never fail unless there is programming error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not resolved, assuming thumbs up means agreement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking on solving it in potentially another PR since the error returned by getting parent groups is outside of scope of this PR, and as far as I remember there are other places throwing the same errors as well. But I have refactored all group related ones.
686d124
to
47f99f5
Compare
pkg/apiserver/registry/networkpolicy/ipassociation/rest_test.go
Outdated
Show resolved
Hide resolved
/test-all |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR description needs to update to reflect the new API path. And the same content should be updated to the commit message (note that only commit message gets into the git log).
Besides, the statement "The API can be used by either querying the kubectl proxy (i.e. curl 127.0.0.1:8001/apis/controlplane.antrea.io/v1beta2/ipassociations/10.10.0.1) or by using the IPAssociations clientset provided under controlplane/v1beta2." may be a bit confusing that they are the only two ways to use the API. However, there are plenty ways to use the API as long as a client can reach the API endpoint and has the authroization. The kubectl proxy is just one manner mainly used in development and the generated code itself doesn't automatic resolving the authorization problem. If this is just a guide for testing, you may talk about querying the kubectl proxy with that path as an usage example.
@@ -393,23 +402,34 @@ func (c *NetworkPolicyController) GetAssociatedGroups(name, namespace string) ([ | |||
|
|||
// getAssociatedGroupsByName retrieves the internal Group and all it's parent Group objects | |||
// (if any) by Group name. | |||
func (c *NetworkPolicyController) getAssociatedGroupsByName(grpName string) []antreatypes.Group { | |||
func (c *NetworkPolicyController) getAssociatedGroupsByName(grpName string) ([]antreatypes.Group, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not resolved, assuming thumbs up means agreement.
This commit adds a new ipgroupassociation API type for querying the Antrea Group/ClusterGroups that an IP is associated with. Possible scenarios include: - IP is assigned to a Pod, in which case the groups that select the Pod as member will be returned - IP appears in an ExternalEntity's endpoints, in which case the groups that select the ExternalEntity as member will be returned - IP is part of an IPBlock that defines the Group/ClusterGroup, in which case the group will be returned. (Note that 1 and 3 can simultaneously be true, so as 2 and 3) Below are examples of how this API can be consumed: - Through kubectl proxy (i.e. curl 127.0.0.1:8001/apis/controlplane .antrea.io/v1beta2/ipassociations/10.10.0.1) - Through the IPGroupAssociations clientset provided under controlplane/v1beta2 Signed-off-by: Dyanngg <dingyang@vmware.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@GraysonWu please let us know if you have other comments |
/test-all |
LGTM |
/test-networkpolicy |
This commit adds a new ipgroupassociation API type for querying the Antrea Group/ClusterGroups that an IP is associated with. Possible scenarios include: - IP is assigned to a Pod, in which case the groups that select the Pod as member will be returned - IP appears in an ExternalEntity's endpoints, in which case the groups that select the ExternalEntity as member will be returned - IP is part of an IPBlock that defines the Group/ClusterGroup, in which case the group will be returned. (Note that 1 and 3 can simultaneously be true, so as 2 and 3) Below are examples of how this API can be consumed: - Through kubectl proxy (i.e. curl 127.0.0.1:8001/apis/controlplane .antrea.io/v1beta2/ipgroupassociations/10.10.0.1) - Through the IPGroupAssociations clientset provided under controlplane/v1beta2 Signed-off-by: Dyanngg <dingyang@vmware.com>
This PR adds a new
ipgroupassociation
API type for querying the Antrea Group/ClusterGroups that an IP is associated with. Possible scenarios include:(Note that 1 and 3 can simultaneously be true, so as 2 and 3)
Below are examples of how this API can be consumed:
kubectl proxy
(e.g.curl 127.0.0.1:8001/apis/controlplane.antrea.io/v1beta2/ipassociations/10.10.0.1
)IPGroupAssociations
clientset provided under controlplane/v1beta2