Obscure message with duplicate ipaddresspools in the pool selector

[fedepaol]

MetalLB Version

latest

Deployment method

Not relevant

Main CNI

not relevant

Kubernetes Version

No response

Cluster Distribution

No response

Describe the bug

Create a bgp advertisement /l2 advertisemetn with the same name twice in the ipaddresspool selector and check the error:

{"caller":"config_controller.go:142","controller":"ConfigReconciler","error":"duplicate definition of ipAddressPools "ingressgateway"","level":"error","ts":"2024-04-17T14:42:54Z"}

To Reproduce

See above

Expected Behavior

A clear message that tells me to remove the duplicate entry from advertisement xy.

I suspect this also applies in other areas where we use the validate* functions.

Additional Context

none

I've read and agree with the following

• I've checked all open and closed issues and my request is not there.
• I've checked all open and closed pull requests and my request is not there.

I've read and agree with the following

• I've checked all open and closed issues and my issue is not there.
• This bug is reproducible when deploying MetalLB from the main branch
• I have read the troubleshooting guide and I am still not able to make it work
• I checked the logs and MetalLB is not discarding the configuration as not valid
• I enabled the debug logs, collected the information required from the cluster using the collect script and will attach them to the issue
• I will provide the definition of my service and the related endpoint slices and attach them to this issue

[tozastation]

hi...!! Can i try to do this issue?

[fedepaol]

sure thing!

[tozastation]

[My Memo]

reproduce manifest

apiVersion: metallb.io/v1beta1
kind: IPAddressPool
metadata:
  name: cheap
  namespace: metallb-system
spec:
  addresses:
  - 192.168.10.0/24
---
apiVersion: metallb.io/v1beta1
kind: L2Advertisement
metadata:
  name: example
  namespace: metallb-system
spec:
  ipAddressPools:
  - cheap
  ipAddressPoolSelectors:
    - matchLabels:
        name: default
    - matchLabels:
        name: default

[tozastation]

[MEMO]

pick up message which contains "duplicate"

• https://github.com/metallb/metallb/blob/main/internal/config/config.go#L261
  • fmt.Errorf("found duplicate bfd profile name %s", parsed.Name)
• https://github.com/metallb/metallb/blob/main/internal/config/config.go#L309
  • fmt.Errorf("duplicate definition of pool %q", p.Name)
• https://github.com/metallb/metallb/blob/main/internal/config/config.go#L360
  • fmt.Errorf("duplicate definition of community %q", communityAlias.Name)
• https://github.com/metallb/metallb/blob/main/internal/config/config.go#L536
  • errors.New("duplicate definition in namespaces field")
  • MEMO: In the project, we are using the package “github.com/pkg/errors”, so we may switch to “fmt.Errorf”.
• https://github.com/metallb/metallb/blob/main/internal/config/config.go#L966
  • fmt.Errorf("duplicate definition of bgpadvertisements. advertisement %d and %d are equal", i+1, j+1)
• https://github.com/metallb/metallb/blob/main/internal/config/config.go#L966
  • fmt.Errorf("duplicate definition of %s %q", labelSelectorType, labelSelectors[i])
• https://github.com/metallb/metallb/blob/main/internal/config/config.go#L1069
  • fmt.Errorf("duplicate definition of %s %q", "match expressions", matchExpressions[i])
• https://github.com/metallb/metallb/blob/main/internal/config/config.go#L1080
  • fmt.Errorf("duplicate definition of %s %q", sliceType, strSlice[i])

The validate function and other functions also have error messages containing duplicate.

Message Candidates:

• Add a statement requesting review or deletion.
  • Duplicate definition of %s %q. Please review or remove duplicates.

Other:
If the Validation Webhook includes this check, the “duplicate definition of” message may be easier to understand without modification?!