nutanix-cloud-native / cluster-api-provider-nutanix

Kubernetes-native declarative infrastructure provider for Nutanix AHV
https://opendocs.nutanix.com/capx/latest/getting_started/
Apache License 2.0
41 stars 22 forks source link

fix(bug): Encode name in filter by name FIQL Query #484

Closed thunderboltsid closed 1 month ago

thunderboltsid commented 1 month ago

Not encoding the name can cause errors when non-standard characters are present in the name.

How has this been tested? Unter testing

Current status: with these changes we are still seeing failed machines with:

  failureMessage: 'Failure detected from referenced resource infrastructure.cluster.x-k8s.io/v1beta1,
    Kind=NutanixMachine with name "quick-start-0j5mcd-zb6l8-k6fqs": failed to retrieve
    subnet by name vlan173|ncn|dev|sandbox'
codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 33.14%. Comparing base (78f61d1) to head (cfbda29).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #484 +/- ## ========================================== + Coverage 33.00% 33.14% +0.14% ========================================== Files 17 17 Lines 1809 1810 +1 ========================================== + Hits 597 600 +3 + Misses 1192 1190 -2 Partials 20 20 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

thunderboltsid commented 1 month ago

Closing this PR as URL encoding doesn't take care of all escaping. e.g. /subnets/list is a POST endpoint and the filter is passed as a json object where the | has to be escaped as \\|. We have decided to make a tradeoff in favor correctness in exchange for performance and do all filtering client-side. We might revisit a more exhaustive FIQL filtering in the future but for now switch to client side filters See #485