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
42 stars 22 forks source link

feat: Validate prism endpoint is a valid address #356

Closed jimmidyson closed 10 months ago

jimmidyson commented 10 months ago

Previously the field could contain ports or schemes, which broke the PC client creation. This commit enforces the value by using OpenAPI JSON schema formats to accept only hostnames, ipv4, or ipv6 addresses.

thunderboltsid commented 10 months ago

/ok-to-test /lgtm

codecov[bot] commented 10 months ago

Codecov Report

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

Comparison is base (1f8716d) 13.62% compared to head (a1e98f9) 11.13%.

:exclamation: Current head a1e98f9 differs from pull request most recent head 3cc26a5. Consider uploading reports for the commit 3cc26a5 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #356 +/- ## ========================================== - Coverage 13.62% 11.13% -2.49% ========================================== Files 4 8 +4 Lines 1013 1266 +253 ========================================== + Hits 138 141 +3 - Misses 875 1125 +250 ```

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

deepakm-ntnx commented 10 months ago

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (085c903) 13.62% compared to head (a1e98f9) 11.13%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #356      +/-   ##
==========================================
- Coverage   13.62%   11.13%   -2.49%     
==========================================
  Files           4        8       +4     
  Lines        1013     1266     +253     
==========================================
+ Hits          138      141       +3     
- Misses        875     1125     +250     

☔ View full report in Codecov by Sentry. 📢 Have feedback on the report? Share it here.

does this mean coverage went down?

thunderboltsid commented 10 months ago

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (085c903) 13.62% compared to head (a1e98f9) 11.13%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #356      +/-   ##
==========================================
- Coverage   13.62%   11.13%   -2.49%     
==========================================
  Files           4        8       +4     
  Lines        1013     1266     +253     
==========================================
+ Hits          138      141       +3     
- Misses        875     1125     +250     

☔ View full report in Codecov by Sentry. 📢 Have feedback on the report? Share it here.

does this mean coverage went down?

yes.

jimmidyson commented 10 months ago

Very confusing to me that code coverage has dropped considering this pr only adds tests (other than non-go-code changes).

thunderboltsid commented 10 months ago

Very confusing to me that code coverage has dropped considering this pr only adds tests (other than non-go-code changes).

my guess is that it brings a package under testing that earlier did not have any tests and as such was ignored for the purpose of coverage tracking.

thunderboltsid commented 10 months ago

Very confusing to me that code coverage has dropped considering this pr only adds tests (other than non-go-code changes).

my guess is that it brings a package under testing that earlier did not have any tests and as such was ignored for the purpose of coverage tracking.

https://app.codecov.io/gh/nutanix-cloud-native/cluster-api-provider-nutanix/commit/a1e98f92069ba514960085d4554f95308aa0937c/indirect-changes

thunderboltsid commented 10 months ago

/approve

thunderboltsid commented 10 months ago

/retest

nutanix-cn-prow-bot[bot] commented 10 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deepakm-ntnx, faiq, jimmidyson, thunderboltsid

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/nutanix-cloud-native/cluster-api-provider-nutanix/blob/main/OWNERS)~~ [deepakm-ntnx,thunderboltsid] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment