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

Make gwctl work with Policies having multiple targetRefs #3196

Open
gauravkghildiyal opened this issue Jul 16, 2024 · 6 comments
Open

Make gwctl work with Policies having multiple targetRefs #3196

gauravkghildiyal opened this issue Jul 16, 2024 · 6 comments
Assignees
Labels
area/gwctl kind/feature Categorizes issue or PR as related to a new feature.

Comments

@gauravkghildiyal
Copy link
Member

As per recent updates to the Policy related GEPs in #2927 (comment), Policies are now allowed to target multiple resources i.e. targetRefs (which previously was limited to a single targetRef).

Make gwctl work with this new pattern. Common places that need to be updated include:

  • Displaying/handling multiple resources appropriately when using gwctl get policies
  • Code changes required within
    func (p Policy) TargetRef() common.ObjRef {
    return p.targetRef
    }
    func (p Policy) IsInherited() bool {
    return p.inherited
    }
    func (p Policy) IsDirect() bool {
    return !p.inherited
    }
    func (p Policy) IsAttachedTo(objRef common.ObjRef) bool {
    if p.targetRef.Kind == "Namespace" && p.targetRef.Name == "" {
    p.targetRef.Name = "default"
    }
    if objRef.Kind == "Namespace" && objRef.Name == "" {
    objRef.Name = "default"
    }
    if p.targetRef.Kind != "Namespace" && p.targetRef.Namespace == "" {
    p.targetRef.Namespace = "default"
    }
    if objRef.Kind != "Namespace" && objRef.Namespace == "" {
    objRef.Namespace = "default"
    }
    return p.targetRef == objRef
    }

/area gwctl
/kind feature

@k8s-ci-robot k8s-ci-robot added area/gwctl kind/feature Categorizes issue or PR as related to a new feature. labels Jul 16, 2024
@deszhou
Copy link
Member

deszhou commented Jul 22, 2024

Can I assign it to myself? I want to try it
@gauravkghildiyal

@gauravkghildiyal
Copy link
Member Author

Hi @deszhou, sure go ahead, though there's a few minor open questions here which may need to be ironed out. Like for example what exactly would the following output:

gwctl get policies

gwctl describe policies

Would be nice if you can give a sample of what you have in mind before diving deep

@deszhou
Copy link
Member

deszhou commented Jul 23, 2024

Hi @deszhou, sure go ahead, though there's a few minor open questions here which may need to be ironed out. Like for example what exactly would the following output:

gwctl get policies

gwctl describe policies

Would be nice if you can give a sample of what you have in mind before diving deep

gwctl get policies -A
  • Indented Format:
NAME                          KIND                       TARGET NAME           TARGET KIND     POLICY TYPE  AGE
health-check-gateway          HealthCheckPolicy.foo.com  foo-gateway           Gateway         Inherited    20d
                                                         foo-gatewayclass      GatewayClass    Inherited    20d                
timeout-policy-gatewayclass   TimeoutPolicy.bar.com      foo-gatewayclass      Gatewayclass    Direct       5m
timeout-policy-httproute      TimeoutPolicy.bar.com      foo-httproute         HTTPRoute       Direct       13m
  • Comma-Separated Format:
NAME                          KIND                       TARGET REFS                                            POLICY TYPE  AGE
health-check-gateway          HealthCheckPolicy.foo.com  foo-gateway (Gateway),foo-gatewayclass (GatewayClass)  Inherited    20d
timeout-policy-gatewayclass   TimeoutPolicy.bar.com      foo-gatewayclass (Gatewayclass)                        Direct       5m
timeout-policy-httproute      TimeoutPolicy.bar.com      foo-httproute (HTTPRoute)                              Direct       13m
gwctl describe policies health-check-gateway
Name: health-check-gateway
Group: foo.com
Kind: HealthCheckPolicy
Inherited: "true"
Spec:
  ...
  targetRefs:
     - group: gateway.networking.k8s.io
        kind: Gateway
        name: foo-gateway
     - group: gateway.networking.k8s.io
        kind: GatewayClass
	name: foo-gatewayclass 

Which of these two output formats do you think is better for gwctl get policies, or do you have any suggestions for improving the output format?

@gauravkghildiyal
Copy link
Member Author

Let's choose the "comma-separated one" which is consistent with others. Let's just display 2 targets and then +n more pattern.

Though the other one also seems good but we can revisit this later.

@deszhou
Copy link
Member

deszhou commented Jul 24, 2024

/assign

@shaneutt
Copy link
Member

@gauravkghildiyal we should transfer this over to the gwctl repository as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/gwctl kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants