From 199317901c0170a0cccc4bccbf84cf138b9b09a2 Mon Sep 17 00:00:00 2001 From: Daniel Lipovetsky Date: Mon, 19 May 2025 17:15:33 -0700 Subject: [PATCH 01/12] feat: Nutanix VM image preflight check --- cmd/main.go | 2 + go.mod | 15 +- go.sum | 49 +++++ pkg/webhook/preflight/nutanix/checker.go | 60 ++++++ pkg/webhook/preflight/nutanix/credentials.go | 205 +++++++++++++++++++ pkg/webhook/preflight/nutanix/image.go | 154 ++++++++++++++ pkg/webhook/preflight/nutanix/specs.go | 91 ++++++++ 7 files changed, 575 insertions(+), 1 deletion(-) create mode 100644 pkg/webhook/preflight/nutanix/checker.go create mode 100644 pkg/webhook/preflight/nutanix/credentials.go create mode 100644 pkg/webhook/preflight/nutanix/image.go create mode 100644 pkg/webhook/preflight/nutanix/specs.go diff --git a/cmd/main.go b/cmd/main.go index d39bc3f49..d498bb023 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -42,6 +42,7 @@ import ( "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/options" "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/cluster" "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight" + preflightnutanix "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight/nutanix" ) func main() { @@ -224,6 +225,7 @@ func main() { Handler: preflight.New(mgr.GetClient(), admission.NewDecoder(mgr.GetScheme()), []preflight.Checker{ // Add your preflight checkers here. + preflightnutanix.New, }..., ), }) diff --git a/go.mod b/go.mod index d9580e851..951d0f879 100644 --- a/go.mod +++ b/go.mod @@ -24,6 +24,7 @@ require ( github.com/nutanix/ntnx-api-golang-clients/clustermgmt-go-client/v4 v4.0.1-beta.2 github.com/nutanix/ntnx-api-golang-clients/networking-go-client/v4 v4.0.2-beta.1 github.com/nutanix/ntnx-api-golang-clients/prism-go-client/v4 v4.0.1-beta.1 + github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4 v4.0.1-beta.1 github.com/onsi/ginkgo/v2 v2.23.4 github.com/onsi/gomega v1.37.0 github.com/pkg/errors v0.9.1 @@ -56,6 +57,8 @@ require ( github.com/Masterminds/semver/v3 v3.3.0 // indirect github.com/Masterminds/sprig/v3 v3.3.0 // indirect github.com/Microsoft/go-winio v0.6.1 // indirect + github.com/PaesslerAG/gval v1.0.0 // indirect + github.com/PaesslerAG/jsonpath v0.1.1 // indirect github.com/ProtonMail/go-crypto v0.0.0-20230217124315-7d5c6f04bbb8 // indirect github.com/adrg/xdg v0.5.3 // indirect github.com/antlr4-go/antlr/v4 v4.13.0 // indirect @@ -78,9 +81,16 @@ require ( github.com/fsnotify/fsnotify v1.8.0 // indirect github.com/fxamacker/cbor/v2 v2.7.0 // indirect github.com/go-logr/stdr v1.2.2 // indirect + github.com/go-logr/zapr v1.3.0 // indirect + github.com/go-openapi/analysis v0.23.0 // indirect + github.com/go-openapi/errors v0.22.0 // indirect github.com/go-openapi/jsonpointer v0.21.0 // indirect github.com/go-openapi/jsonreference v0.21.0 // indirect + github.com/go-openapi/loads v0.22.0 // indirect + github.com/go-openapi/spec v0.21.0 // indirect + github.com/go-openapi/strfmt v0.23.0 // indirect github.com/go-openapi/swag v0.23.0 // indirect + github.com/go-openapi/validate v0.24.0 // indirect github.com/go-task/slim-sprig/v3 v3.0.0 // indirect github.com/go-viper/mapstructure/v2 v2.2.1 // indirect github.com/gogo/protobuf v1.3.2 // indirect @@ -106,14 +116,15 @@ require ( github.com/mattn/go-isatty v0.0.20 // indirect github.com/mattn/go-runewidth v0.0.14 // indirect github.com/mitchellh/copystructure v1.2.0 // indirect + github.com/mitchellh/mapstructure v1.5.0 // indirect github.com/mitchellh/reflectwalk v1.0.2 // indirect github.com/moby/docker-image-spec v1.3.1 // indirect github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.2 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/nutanix/ntnx-api-golang-clients/storage-go-client/v4 v4.0.2-alpha.3 // indirect - github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4 v4.0.1-beta.1 // indirect github.com/nutanix/ntnx-api-golang-clients/volumes-go-client/v4 v4.0.1-beta.1 // indirect + github.com/oklog/ulid v1.3.1 // indirect github.com/olekukonko/tablewriter v0.0.5 // indirect github.com/opencontainers/go-digest v1.0.0 // indirect github.com/opencontainers/image-spec v1.1.0-rc5 // indirect @@ -137,6 +148,7 @@ require ( github.com/subosito/gotenv v1.6.0 // indirect github.com/valyala/fastjson v1.6.4 // indirect github.com/x448/float16 v0.8.4 // indirect + go.mongodb.org/mongo-driver v1.14.0 // indirect go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.54.0 // indirect go.opentelemetry.io/otel v1.29.0 // indirect go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.28.0 // indirect @@ -147,6 +159,7 @@ require ( go.opentelemetry.io/proto/otlp v1.3.1 // indirect go.uber.org/automaxprocs v1.6.0 // indirect go.uber.org/multierr v1.11.0 // indirect + go.uber.org/zap v1.27.0 // indirect golang.org/x/crypto v0.36.0 // indirect golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 // indirect golang.org/x/mod v0.24.0 // indirect diff --git a/go.sum b/go.sum index 1446a360b..cc35a739d 100644 --- a/go.sum +++ b/go.sum @@ -18,12 +18,19 @@ github.com/Masterminds/sprig/v3 v3.3.0 h1:mQh0Yrg1XPo6vjYXgtf5OtijNAKJRNcTdOOGZe github.com/Masterminds/sprig/v3 v3.3.0/go.mod h1:Zy1iXRYNqNLUolqCpL4uhk6SHUMAOSCzdgBfDb35Lz0= github.com/Microsoft/go-winio v0.6.1 h1:9/kr64B9VUZrLm5YYwbGtUJnMgqWVOdUAXu6Migciow= github.com/Microsoft/go-winio v0.6.1/go.mod h1:LRdKpFKfdobln8UmuiYcKPot9D2v6svN5+sAH+4kjUM= +github.com/PaesslerAG/gval v1.0.0 h1:GEKnRwkWDdf9dOmKcNrar9EA1bz1z9DqPIO1+iLzhd8= +github.com/PaesslerAG/gval v1.0.0/go.mod h1:y/nm5yEyTeX6av0OfKJNp9rBNj2XrGhAf5+v24IBN1I= +github.com/PaesslerAG/jsonpath v0.1.0/go.mod h1:4BzmtoM/PI8fPO4aQGIusjGxGir2BzcV0grWtFzq1Y8= +github.com/PaesslerAG/jsonpath v0.1.1 h1:c1/AToHQMVsduPAa4Vh6xp2U0evy4t8SWp8imEsylIk= +github.com/PaesslerAG/jsonpath v0.1.1/go.mod h1:lVboNxFGal/VwW6d9JzIy56bUsYAP6tH/x80vjnCseY= github.com/ProtonMail/go-crypto v0.0.0-20230217124315-7d5c6f04bbb8 h1:wPbRQzjjwFc0ih8puEVAOFGELsn1zoIIYdxvML7mDxA= github.com/ProtonMail/go-crypto v0.0.0-20230217124315-7d5c6f04bbb8/go.mod h1:I0gYDMZ6Z5GRU7l58bNFSkPTFN6Yl12dsUlAZ8xy98g= github.com/adrg/xdg v0.5.3 h1:xRnxJXne7+oWDatRhR1JLnvuccuIeCoBu2rtuLqQB78= github.com/adrg/xdg v0.5.3/go.mod h1:nlTsY+NNiCBGCK2tpm09vRqfVzrc2fLmXGpBLF0zlTQ= github.com/antlr4-go/antlr/v4 v4.13.0 h1:lxCg3LAv+EUK6t1i0y1V6/SLeUi0eKEKdhQAlS8TVTI= github.com/antlr4-go/antlr/v4 v4.13.0/go.mod h1:pfChB/xh/Unjila75QW7+VU4TSnWnnk9UTnmpPaOR2g= +github.com/araddon/dateparse v0.0.0-20210429162001-6b43995a97de h1:FxWPpzIjnTlhPwqqXc4/vE0f7GvRjuAsbW+HOIe8KnA= +github.com/araddon/dateparse v0.0.0-20210429162001-6b43995a97de/go.mod h1:DCaWoUhZrYW9p1lxo/cm8EmUOOzAPSEZNGF2DK1dJgw= github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 h1:DklsrG3dyBCFEj5IhUbnKptjxatkF07cF2ak3yi77so= github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2/go.mod h1:WaHUgvxTVq04UNunO+XhnAqY/wQc+bxr74GqbsZ/Jqw= github.com/aws/aws-sdk-go v1.55.7 h1:UJrkFq7es5CShfBwlWAC8DA077vp8PyVbQd3lqLiztE= @@ -52,6 +59,8 @@ github.com/coreos/go-systemd v0.0.0-20191104093116-d3cd4ed1dbcf h1:iW4rZ826su+pq github.com/coreos/go-systemd/v22 v22.5.0 h1:RrqgGjYQKalulkV8NGVIfkXQf6YYmOyiJKk8iXXhfZs= github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc= github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= +github.com/creasty/defaults v1.6.0 h1:ltuE9cfphUtlrBeomuu8PEyISTXnxqkBIoQfXgv7BSc= +github.com/creasty/defaults v1.6.0/go.mod h1:iGzKe6pbEHnpMPtfDXZEr0NVxWnPTjb1bbDy08fPzYM= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= @@ -80,6 +89,8 @@ github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHk github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= github.com/fsnotify/fsnotify v1.8.0 h1:dAwr6QBTBZIkG8roQaJjGof0pp0EeF+tNV7YBP3F/8M= github.com/fsnotify/fsnotify v1.8.0/go.mod h1:8jBTzvmWwFyi3Pb8djgCCO5IBqzKJ/Jwo8TRcHyHii0= +github.com/fullstorydev/grpcurl v1.8.7 h1:xJWosq3BQovQ4QrdPO72OrPiWuGgEsxY8ldYsJbPrqI= +github.com/fullstorydev/grpcurl v1.8.7/go.mod h1:pVtM4qe3CMoLaIzYS8uvTuDj2jVYmXqMUkZeijnXp/E= github.com/fxamacker/cbor/v2 v2.7.0 h1:iM5WgngdRBanHcxugY4JySA0nk1wZorNOpTgCMedv5E= github.com/fxamacker/cbor/v2 v2.7.0/go.mod h1:pxXPTn3joSm21Gbwsv0w9OSA2y1HFR9qXEeXQVeNoDQ= github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= @@ -89,14 +100,34 @@ github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= github.com/go-logr/zapr v1.3.0 h1:XGdV8XW8zdwFiwOA2Dryh1gj2KRQyOOoNmBy4EplIcQ= github.com/go-logr/zapr v1.3.0/go.mod h1:YKepepNBd1u/oyhd/yQmtjVXmm9uML4IXUgMOwR8/Gg= +github.com/go-openapi/analysis v0.23.0 h1:aGday7OWupfMs+LbmLZG4k0MYXIANxcuBTYUC03zFCU= +github.com/go-openapi/analysis v0.23.0/go.mod h1:9mz9ZWaSlV8TvjQHLl2mUW2PbZtemkE8yA5v22ohupo= +github.com/go-openapi/errors v0.22.0 h1:c4xY/OLxUBSTiepAg3j/MHuAv5mJhnf53LLMWFB+u/w= +github.com/go-openapi/errors v0.22.0/go.mod h1:J3DmZScxCDufmIMsdOuDHxJbdOGC0xtUynjIx092vXE= github.com/go-openapi/jsonpointer v0.21.0 h1:YgdVicSA9vH5RiHs9TZW5oyafXZFc6+2Vc1rr/O9oNQ= github.com/go-openapi/jsonpointer v0.21.0/go.mod h1:IUyH9l/+uyhIYQ/PXVA41Rexl+kOkAPDdXEYns6fzUY= github.com/go-openapi/jsonreference v0.21.0 h1:Rs+Y7hSXT83Jacb7kFyjn4ijOuVGSvOdF2+tg1TRrwQ= github.com/go-openapi/jsonreference v0.21.0/go.mod h1:LmZmgsrTkVg9LG4EaHeY8cBDslNPMo06cago5JNLkm4= +github.com/go-openapi/loads v0.22.0 h1:ECPGd4jX1U6NApCGG1We+uEozOAvXvJSF4nnwHZ8Aco= +github.com/go-openapi/loads v0.22.0/go.mod h1:yLsaTCS92mnSAZX5WWoxszLj0u+Ojl+Zs5Stn1oF+rs= +github.com/go-openapi/spec v0.21.0 h1:LTVzPc3p/RzRnkQqLRndbAzjY0d0BCL72A6j3CdL9ZY= +github.com/go-openapi/spec v0.21.0/go.mod h1:78u6VdPw81XU44qEWGhtr982gJ5BWg2c0I5XwVMotYk= +github.com/go-openapi/strfmt v0.23.0 h1:nlUS6BCqcnAk0pyhi9Y+kdDVZdZMHfEKQiS4HaMgO/c= +github.com/go-openapi/strfmt v0.23.0/go.mod h1:NrtIpfKtWIygRkKVsxh7XQMDQW5HKQl6S5ik2elW+K4= github.com/go-openapi/swag v0.23.0 h1:vsEVJDUo2hPJ2tu0/Xc+4noaxyEffXNIs3cOULZ+GrE= github.com/go-openapi/swag v0.23.0/go.mod h1:esZ8ITTYEsH1V2trKHjAN8Ai7xHb8RV+YSZ577vPjgQ= +github.com/go-openapi/validate v0.24.0 h1:LdfDKwNbpB6Vn40xhTdNZAnfLECL81w+VX3BumrGD58= +github.com/go-openapi/validate v0.24.0/go.mod h1:iyeX1sEufmv3nPbBdX3ieNviWnOZaJ1+zquzJEf2BAQ= +github.com/go-playground/locales v0.14.0 h1:u50s323jtVGugKlcYeyzC0etD1HifMjqmJqb8WugfUU= +github.com/go-playground/locales v0.14.0/go.mod h1:sawfccIbzZTqEDETgFXqTho0QybSa7l++s0DH+LDiLs= +github.com/go-playground/universal-translator v0.18.0 h1:82dyy6p4OuJq4/CByFNOn/jYrnRPArHwAcmLoJZxyho= +github.com/go-playground/universal-translator v0.18.0/go.mod h1:UvRDBj+xPUEGrFYl+lu/H90nyDXpg0fqeB/AQUGNTVA= +github.com/go-playground/validator/v10 v10.10.1 h1:uA0+amWMiglNZKZ9FJRKUAe9U3RX91eVn1JYXMWt7ig= +github.com/go-playground/validator/v10 v10.10.1/go.mod h1:i+3WkQ1FvaUjjxh1kSvIA4dMGDBiPU55YFDl0WbKdWU= github.com/go-task/slim-sprig/v3 v3.0.0 h1:sUs3vkvUymDpBKi3qH1YSqBQk9+9D/8M2mN1vB6EwHI= github.com/go-task/slim-sprig/v3 v3.0.0/go.mod h1:W848ghGpv3Qj3dhTPRyJypKRiqCdHZiAzKg9hl15HA8= +github.com/go-test/deep v1.0.8 h1:TDsG77qcSprGbC6vTN8OuXp5g+J+b5Pcguhf7Zt61VM= +github.com/go-test/deep v1.0.8/go.mod h1:5C2ZWiW0ErCdrYzpqxLbTX7MG14M9iiw8DgHncVwcsE= github.com/go-viper/mapstructure/v2 v2.2.1 h1:ZAaOCxANMuZx5RCeg0mBdEZk7DZasvvZIxtHqx8aGss= github.com/go-viper/mapstructure/v2 v2.2.1/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM= github.com/gobuffalo/flect v1.0.3 h1:xeWBM2nui+qnVvNM4S3foBhCAL2XgPU+a7FdpelbTq4= @@ -146,6 +177,8 @@ github.com/huandu/xstrings v1.5.0 h1:2ag3IFq9ZDANvthTwTiqSSZLjDc+BedvHPAp5tJy2TI github.com/huandu/xstrings v1.5.0/go.mod h1:y5/lhBue+AyNmUVz9RLU9xbLR0o4KIIExikq4ovT0aE= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= +github.com/jhump/protoreflect v1.14.0 h1:MBbQK392K3u8NTLbKOCIi3XdI+y+c6yt5oMq0X3xviw= +github.com/jhump/protoreflect v1.14.0/go.mod h1:JytZfP5d0r8pVNLZvai7U/MCuTWITgrI4tTg7puQFKI= github.com/jmespath/go-jmespath v0.4.0 h1:BEgLn5cpjn8UN1mAw4NjwDrS35OdebyEtFe+9YPoQUg= github.com/jmespath/go-jmespath v0.4.0/go.mod h1:T8mJZnbsbmF+m6zOOFylbeCJqk5+pHWvzYPziyZiYoo= github.com/jmespath/go-jmespath/internal/testify v1.5.1 h1:shLQSRRSCCPj3f2gpwzGwWFoC7ycTf1rcQZHOlsJ6N8= @@ -154,12 +187,18 @@ github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8Hm github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y= github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM= github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= +github.com/k0kubun/pp/v3 v3.1.0 h1:ifxtqJkRZhw3h554/z/8zm6AAbyO4LLKDlA5eV+9O8Q= +github.com/k0kubun/pp/v3 v3.1.0/go.mod h1:vIrP5CF0n78pKHm2Ku6GVerpZBJvscg48WepUYEk2gw= +github.com/keploy/go-sdk v0.9.0 h1:kpSNcCTDdELsa1gWyhoD9oV57SgSMbG/wq6Cjp4y7cY= +github.com/keploy/go-sdk v0.9.0/go.mod h1:vNKXoFd2MaK+Gly/K6XeP1Hs9dP834C74szH+vtBPwg= github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= +github.com/leodido/go-urn v1.2.1 h1:BqpAaACuzVSgi/VLzGZIobT2z4v53pjosyNd9Yv6n/w= +github.com/leodido/go-urn v1.2.1/go.mod h1:zt4jvISO2HfUBqxjfIshjdMTYS56ZS/qv49ictyFfxY= github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0= github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc= github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA= @@ -172,6 +211,8 @@ github.com/mattn/go-runewidth v0.0.14 h1:+xnbZSEeDbOIg5/mE6JF0w6n9duR1l3/WmbinWV github.com/mattn/go-runewidth v0.0.14/go.mod h1:Jdepj2loyihRzMpdS35Xk/zdY8IAYHsh153qUoGf23w= github.com/mitchellh/copystructure v1.2.0 h1:vpKXTN4ewci03Vljg/q9QvCGUDttBOGBIa15WveJJGw= github.com/mitchellh/copystructure v1.2.0/go.mod h1:qLl+cE2AmVv+CoeAwDPye/v+N2HKCj9FbZEVFJRxO9s= +github.com/mitchellh/mapstructure v1.5.0 h1:jeMsZIYE/09sWLaz43PL7Gy6RuMjD2eJVyuac5Z2hdY= +github.com/mitchellh/mapstructure v1.5.0/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo= github.com/mitchellh/reflectwalk v1.0.2 h1:G2LzWKi524PWgd3mLHV8Y5k7s6XUvT0Gef6zxSIeXaQ= github.com/mitchellh/reflectwalk v1.0.2/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw= github.com/moby/docker-image-spec v1.3.1 h1:jMKff3w6PgbfSa69GfNg+zN/XLhfXJGnEx3Nl2EsFP0= @@ -201,6 +242,8 @@ github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4 v4.0.1-beta.1 h1:XuT github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4 v4.0.1-beta.1/go.mod h1:CaWm4GFpAjQQDc6YXl/dUDrHpuW54h8j6Cj7EslE4Qk= github.com/nutanix/ntnx-api-golang-clients/volumes-go-client/v4 v4.0.1-beta.1 h1:VJSaQDnnYeNEk1mkQqEbt573OdM62+5s/B0e9kszdas= github.com/nutanix/ntnx-api-golang-clients/volumes-go-client/v4 v4.0.1-beta.1/go.mod h1:Z+RKLwsHYxAcFbZPy2ft3QAK9kBPt9bQdqXSp7eYWkY= +github.com/oklog/ulid v1.3.1 h1:EGfNDEx6MqHz8B3uNV6QAib1UR2Lm97sHi3ocA6ESJ4= +github.com/oklog/ulid v1.3.1/go.mod h1:CirwcVhetQ6Lv90oh/F+FBtV6XMibvdAFo93nm5qn4U= github.com/olekukonko/tablewriter v0.0.5 h1:P2Ga83D34wi1o9J6Wh1mRuqd4mF/x/lgBS7N7AbDhec= github.com/olekukonko/tablewriter v0.0.5/go.mod h1:hPp6KlRPjbx+hW8ykQs1w3UBbZlj6HuIJcUGPhkA7kY= github.com/onsi/ginkgo/v2 v2.23.4 h1:ktYTpKJAVZnDT4VjxSbiBenUjmlL/5QkBEocaWXiQus= @@ -283,6 +326,10 @@ go.etcd.io/etcd/client/pkg/v3 v3.5.20 h1:sZIAtra+xCo56gdf6BR62to/hiie5Bwl7hQIqMz go.etcd.io/etcd/client/pkg/v3 v3.5.20/go.mod h1:qaOi1k4ZA9lVLejXNvyPABrVEe7VymMF2433yyRQ7O0= go.etcd.io/etcd/client/v3 v3.5.20 h1:jMT2MwQEhyvhQg49Cec+1ZHJzfUf6ZgcmV0GjPv0tIQ= go.etcd.io/etcd/client/v3 v3.5.20/go.mod h1:J5lbzYRMUR20YolS5UjlqqMcu3/wdEvG5VNBhzyo3m0= +go.keploy.io/server v0.8.6 h1:czE9jaliyAkMMJcYnMPNuu6tun7UgwFbokxEG95vLN4= +go.keploy.io/server v0.8.6/go.mod h1:t7BPuZQSiC3PNHZ9dbn3e3VB61HNWwiqVmaRujfDFUg= +go.mongodb.org/mongo-driver v1.14.0 h1:P98w8egYRjYe3XDjxhYJagTokP/H6HzlsnojRgZRd80= +go.mongodb.org/mongo-driver v1.14.0/go.mod h1:Vzb0Mk/pa7e6cWw85R4F/endUC3u0U9jGcNU603k65c= go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.54.0 h1:r6I7RJCN86bpD/FQwedZ0vSixDpwuWREjW9oRMsmqDc= go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.54.0/go.mod h1:B9yO6b04uB80CzjedvewuqDhxJxi11s7/GtiGa8bAjI= go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.54.0 h1:TT4fX+nBOA/+LUkobKGW1ydGcn+G3vRw9+g5HwCphpk= @@ -369,6 +416,8 @@ golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8T golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gomodules.xyz/jsonpatch/v2 v2.5.0 h1:JELs8RLM12qJGXU4u/TO3V25KW8GreMKl9pdkk14RM0= gomodules.xyz/jsonpatch/v2 v2.5.0/go.mod h1:AH3dM2RI6uoBZxn3LVrfvJ3E0/9dG4cSrbuBJT4moAY= +google.golang.org/genproto v0.0.0-20241118233622-e639e219e697 h1:ToEetK57OidYuqD4Q5w+vfEnPvPpuTwedCNVohYJfNk= +google.golang.org/genproto v0.0.0-20241118233622-e639e219e697/go.mod h1:JJrvXBWRZaFMxBufik1a4RpFw4HhgVtBBWQeQgUj2cc= google.golang.org/genproto/googleapis/api v0.0.0-20241209162323-e6fa225c2576 h1:CkkIfIt50+lT6NHAVoRYEyAvQGFM7xEwXUUywFvEb3Q= google.golang.org/genproto/googleapis/api v0.0.0-20241209162323-e6fa225c2576/go.mod h1:1R3kvZ1dtP3+4p4d3G8uJ8rFk/fWlScl38vanWACI08= google.golang.org/genproto/googleapis/rpc v0.0.0-20241223144023-3abc09e42ca8 h1:TqExAhdPaB60Ux47Cn0oLV07rGnxZzIsaRhQaqS666A= diff --git a/pkg/webhook/preflight/nutanix/checker.go b/pkg/webhook/preflight/nutanix/checker.go new file mode 100644 index 000000000..c6c7a7f8a --- /dev/null +++ b/pkg/webhook/preflight/nutanix/checker.go @@ -0,0 +1,60 @@ +// Copyright 2025 Nutanix. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package nutanix + +import ( + "context" + + "github.com/go-logr/logr" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + ctrl "sigs.k8s.io/controller-runtime" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + + prismgoclient "github.com/nutanix-cloud-native/prism-go-client" + prismv3 "github.com/nutanix-cloud-native/prism-go-client/v3" + prismv4 "github.com/nutanix-cloud-native/prism-go-client/v4" + + carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" + "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight" +) + +func New(kclient ctrlclient.Client, cluster *clusterv1.Cluster) preflight.Checker { + return &nutanixChecker{ + kclient: kclient, + cluster: cluster, + } +} + +type nutanixChecker struct { + kclient ctrlclient.Client + cluster *clusterv1.Cluster + + nutanixClusterConfigSpec *carenv1.NutanixClusterConfigSpec + nutanixWorkerNodeConfigSpecByMachineDeploymentName map[string]*carenv1.NutanixWorkerNodeConfigSpec + + credentials prismgoclient.Credentials + v3client *prismv3.Client + v4client *prismv4.Client + + log logr.Logger +} + +func (n *nutanixChecker) Init( + ctx context.Context, +) []preflight.Check { + n.log = ctrl.LoggerFrom(ctx).WithName("preflight/nutanix") + + checks := []preflight.Check{ + // The configuration check must run first, because it initializes data used by all other checks, + // and the credentials check second, because it initializes the Nutanix clients used by other checks. + n.initNutanixConfiguration(), + n.initCredentialsCheck(ctx), + } + + checks = append(checks, n.initVMImageChecks()...) + + // Add more checks here as needed. + + return checks +} diff --git a/pkg/webhook/preflight/nutanix/credentials.go b/pkg/webhook/preflight/nutanix/credentials.go new file mode 100644 index 000000000..2464ad143 --- /dev/null +++ b/pkg/webhook/preflight/nutanix/credentials.go @@ -0,0 +1,205 @@ +// Copyright 2025 Nutanix. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package nutanix + +import ( + "context" + "fmt" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" + + prismgoclient "github.com/nutanix-cloud-native/prism-go-client" + prismcredentials "github.com/nutanix-cloud-native/prism-go-client/environment/credentials" + prismv3 "github.com/nutanix-cloud-native/prism-go-client/v3" + prismv4 "github.com/nutanix-cloud-native/prism-go-client/v4" + + "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight" +) + +const credentialsSecretDataKey = "credentials" + +func (n *nutanixChecker) initCredentialsCheck(ctx context.Context) preflight.Check { + n.log.V(5).Info("Initializing Nutanix credentials check") + + result := preflight.CheckResult{ + Name: "NutanixCredentials", + Allowed: true, + } + + if n.nutanixClusterConfigSpec == nil && len(n.nutanixWorkerNodeConfigSpecByMachineDeploymentName) == 0 { + // If there is no Nutanix configuration at all, the credentials check is not needed. + return func(ctx context.Context) preflight.CheckResult { + return result + } + } + + // There is some Nutanix configuration, so the credentials check is needed. + // However, the credentials configuration is missing, so we cannot perform the check. + if n.nutanixClusterConfigSpec == nil || n.nutanixClusterConfigSpec.Nutanix == nil { + result.Allowed = false + result.Error = true + result.Causes = append(result.Causes, + preflight.Cause{ + Message: "Nutanix cluster configuration is not defined in the cluster spec", + Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix", + }, + ) + return func(ctx context.Context) preflight.CheckResult { + return result + } + } + + // Get the credentials data in order to initialize the credentials and clients. + prismCentralEndpointSpec := n.nutanixClusterConfigSpec.Nutanix.PrismCentralEndpoint + + host, port, err := prismCentralEndpointSpec.ParseURL() + if err != nil { + // Should not happen if the cluster passed CEL validation rules. + result.Allowed = false + result.Error = true + result.Causes = append(result.Causes, + preflight.Cause{ + Message: fmt.Sprintf("failed to parse Prism Central endpoint URL: %s", err), + Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.url", + }, + ) + return func(ctx context.Context) preflight.CheckResult { + return result + } + } + + credentialsSecret := &corev1.Secret{} + err = n.kclient.Get( + ctx, + types.NamespacedName{ + Namespace: n.cluster.Namespace, + Name: prismCentralEndpointSpec.Credentials.SecretRef.Name, + }, + credentialsSecret, + ) + if err != nil { + result.Allowed = false + result.Error = true + result.Causes = append(result.Causes, + preflight.Cause{ + Message: fmt.Sprintf("failed to get Prism Central credentials Secret: %s", err), + Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials.secretRef", + }, + ) + return func(ctx context.Context) preflight.CheckResult { + return result + } + } + + if len(credentialsSecret.Data) == 0 { + result.Allowed = false + result.Error = true + result.Causes = append(result.Causes, + preflight.Cause{ + Message: fmt.Sprintf( + "credentials Secret '%s' is empty", + prismCentralEndpointSpec.Credentials.SecretRef.Name, + ), + Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials.secretRef", + }, + ) + return func(ctx context.Context) preflight.CheckResult { + return result + } + } + + data, ok := credentialsSecret.Data[credentialsSecretDataKey] + if !ok { + result.Allowed = false + result.Error = true + result.Causes = append(result.Causes, + preflight.Cause{ + Message: fmt.Sprintf( + "credentials Secret '%s' does not contain key '%s'", + prismCentralEndpointSpec.Credentials.SecretRef.Name, + credentialsSecretDataKey, + ), + Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials.secretRef", + }, + ) + return func(ctx context.Context) preflight.CheckResult { + return result + } + } + + usernamePassword, err := prismcredentials.ParseCredentials(data) + if err != nil { + result.Allowed = false + result.Error = true + result.Causes = append(result.Causes, + preflight.Cause{ + Message: fmt.Sprintf("failed to parse Prism Central credentials: %s", err), + Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials", + }, + ) + return func(ctx context.Context) preflight.CheckResult { + return result + } + } + + if !result.Allowed || result.Error { + // If any error has happened, we should not try to initialize the credentials or clients, so we return early. + return func(ctx context.Context) preflight.CheckResult { + return result + } + } + + // Initialize the credentials. + n.credentials = prismgoclient.Credentials{ + Endpoint: fmt.Sprintf("%s:%d", host, port), + URL: fmt.Sprintf("https://%s:%d", host, port), + Username: usernamePassword.Username, + Password: usernamePassword.Password, + } + if prismCentralEndpointSpec.Insecure { + n.credentials.Insecure = true + n.credentials.URL = fmt.Sprintf("http://%s:%d", host, port) + } + + // Initialize the clients. + n.v4client, err = prismv4.NewV4Client(n.credentials) + if err != nil { + result.Allowed = false + result.Error = true + result.Causes = append(result.Causes, + preflight.Cause{ + Message: fmt.Sprintf("failed to initialize Nutanix V4 client: %s", err), + Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials", + }, + ) + } + + n.v3client, err = prismv3.NewV3Client(n.credentials) + if err != nil { + result.Allowed = false + result.Error = true + result.Causes = append(result.Causes, + preflight.Cause{ + Message: fmt.Sprintf("failed to initialize Nutanix V3 client: %s", err), + Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials", + }, + ) + } + _, err = n.v3client.V3.GetCurrentLoggedInUser(ctx) + if err != nil { + result.Allowed = false + result.Error = true + result.Causes = append(result.Causes, + preflight.Cause{ + Message: fmt.Sprintf("failed to validate credentials using the v3 API client: %s", err), + Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials", + }, + ) + } + + return func(ctx context.Context) preflight.CheckResult { + return result + } +} diff --git a/pkg/webhook/preflight/nutanix/image.go b/pkg/webhook/preflight/nutanix/image.go new file mode 100644 index 000000000..4af21a44f --- /dev/null +++ b/pkg/webhook/preflight/nutanix/image.go @@ -0,0 +1,154 @@ +// Copyright 2025 Nutanix. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package nutanix + +import ( + "context" + "fmt" + + vmmv4 "github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4/models/vmm/v4/content" + + prismv4 "github.com/nutanix-cloud-native/prism-go-client/v4" + + capxv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/external/github.com/nutanix-cloud-native/cluster-api-provider-nutanix/api/v1beta1" + carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" + "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight" +) + +func (n *nutanixChecker) initVMImageChecks() []preflight.Check { + checks := []preflight.Check{} + + if n.nutanixClusterConfigSpec != nil && n.nutanixClusterConfigSpec.ControlPlane != nil && + n.nutanixClusterConfigSpec.ControlPlane.Nutanix != nil { + checks = append(checks, + n.vmImageCheck( + &n.nutanixClusterConfigSpec.ControlPlane.Nutanix.MachineDetails, + "cluster.spec.topology[.name=clusterConfig].value.controlPlane.nutanix.machineDetails", + ), + ) + } + + for mdName, nutanixWorkerNodeConfigSpec := range n.nutanixWorkerNodeConfigSpecByMachineDeploymentName { + if nutanixWorkerNodeConfigSpec.Nutanix != nil { + checks = append(checks, + n.vmImageCheck( + &nutanixWorkerNodeConfigSpec.Nutanix.MachineDetails, + fmt.Sprintf( + "cluster.spec.topology.workers.machineDeployments[.name=%s]"+ + ".variables[.name=workerConfig].value.nutanix.machineDetails", + mdName, + ), + ), + ) + } + } + + return checks +} + +func (n *nutanixChecker) vmImageCheck( + machineDetails *carenv1.NutanixMachineDetails, + field string, +) preflight.Check { + n.log.V(5).Info("Initializing Nutanix VM image check", "field", field) + + return func(ctx context.Context) preflight.CheckResult { + result := preflight.CheckResult{ + Name: "NutanixVMImage", + Allowed: false, + } + + // If the v4 client is not initialized, we cannot perform VM image checks. + if n.v4client == nil { + result.Allowed = false + result.Error = true + result.Causes = append(result.Causes, + preflight.Cause{ + Message: "Nutanix v4 client is not initialized, cannot perform VM image checks", + Field: "", + }, + ) + return result + } + + if machineDetails.ImageLookup != nil { + result.Allowed = false + result.Error = true + result.Causes = append(result.Causes, preflight.Cause{ + Message: "ImageLookup is not yet supported", + Field: field, + }) + return result + } + + if machineDetails.Image != nil { + imagesCh := make(chan []vmmv4.Image) + defer close(imagesCh) + errCh := make(chan error) + defer close(errCh) + + images, err := getVMImages(n.v4client, machineDetails.Image) + if err != nil { + result.Allowed = false + result.Error = true + result.Causes = append(result.Causes, preflight.Cause{ + Message: fmt.Sprintf("failed to get VM Image: %s", err), + Field: field, + }) + return result + } + + if len(images) != 1 { + result.Allowed = false + result.Causes = append(result.Causes, preflight.Cause{ + Message: fmt.Sprintf("expected to find 1 VM Image, found %d", len(images)), + Field: field, + }) + return result + } + + // Found exactly one image. + result.Allowed = true + return result + } + + // Neither ImageLookup nor Image is specified. + return result + } +} + +func getVMImages( + client *prismv4.Client, + id *capxv1.NutanixResourceIdentifier, +) ([]vmmv4.Image, error) { + switch { + case id.IsUUID(): + resp, err := client.ImagesApiInstance.GetImageById(id.UUID) + if err != nil { + return nil, err + } + image, ok := resp.GetData().(vmmv4.Image) + if !ok { + return nil, fmt.Errorf("failed to get data returned by GetImageById") + } + return []vmmv4.Image{image}, nil + case id.IsName(): + filter_ := fmt.Sprintf("name eq '%s'", *id.Name) + resp, err := client.ImagesApiInstance.ListImages(nil, nil, &filter_, nil, nil) + if err != nil { + return nil, err + } + if resp == nil || resp.GetData() == nil { + // No images were returned. + return []vmmv4.Image{}, nil + } + images, ok := resp.GetData().([]vmmv4.Image) + if !ok { + return nil, fmt.Errorf("failed to get data returned by ListImages") + } + return images, nil + default: + return nil, fmt.Errorf("image identifier is missing both name and uuid") + } +} diff --git a/pkg/webhook/preflight/nutanix/specs.go b/pkg/webhook/preflight/nutanix/specs.go new file mode 100644 index 000000000..78796223f --- /dev/null +++ b/pkg/webhook/preflight/nutanix/specs.go @@ -0,0 +1,91 @@ +// Copyright 2025 Nutanix. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package nutanix + +import ( + "context" + "fmt" + + carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" + "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/variables" + "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight" +) + +func (n *nutanixChecker) initNutanixConfiguration() preflight.Check { + n.log.V(5).Info("Initializing Nutanix configuration check") + + result := preflight.CheckResult{ + Name: "NutanixCredentials", + Allowed: true, + } + + nutanixClusterConfigSpec := &carenv1.NutanixClusterConfigSpec{} + err := variables.UnmarshalClusterVariable( + variables.GetClusterVariableByName( + carenv1.ClusterConfigVariableName, + n.cluster.Spec.Topology.Variables, + ), + nutanixClusterConfigSpec, + ) + if err != nil { + // Should not happen if the cluster passed CEL validation rules. + result.Allowed = false + result.Error = true + result.Causes = append(result.Causes, + preflight.Cause{ + Message: fmt.Sprintf("Failed to unmarshal cluster variable %s: %s", + carenv1.ClusterConfigVariableName, + err, + ), + Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix", + }, + ) + } + + // Save the NutanixClusterConfigSpec only if it contains Nutanix configuration. + if nutanixClusterConfigSpec.Nutanix != nil || + (nutanixClusterConfigSpec.ControlPlane != nil && nutanixClusterConfigSpec.ControlPlane.Nutanix != nil) { + n.nutanixClusterConfigSpec = nutanixClusterConfigSpec + } + + nutanixWorkerNodeConfigSpecByMachineDeploymentName := make(map[string]*carenv1.NutanixWorkerNodeConfigSpec) + if n.cluster.Spec.Topology.Workers != nil { + for _, md := range n.cluster.Spec.Topology.Workers.MachineDeployments { + if md.Variables == nil { + continue + } + nutanixWorkerNodeConfigSpec := &carenv1.NutanixWorkerNodeConfigSpec{} + err := variables.UnmarshalClusterVariable( + variables.GetClusterVariableByName(carenv1.WorkerConfigVariableName, md.Variables.Overrides), + nutanixWorkerNodeConfigSpec, + ) + if err != nil { + // Should not happen if the cluster passed CEL validation rules. + result.Allowed = false + result.Error = true + result.Causes = append(result.Causes, + preflight.Cause{ + Message: fmt.Sprintf("Failed to unmarshal topology machineDeployment variable %s: %s", + carenv1.WorkerConfigVariableName, + err, + ), + Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix", + }, + ) + } + // Save the NutanixWorkerNodeConfigSpec only if it contains Nutanix configuration. + if nutanixWorkerNodeConfigSpec.Nutanix != nil { + nutanixWorkerNodeConfigSpecByMachineDeploymentName[md.Name] = nutanixWorkerNodeConfigSpec + } + } + } + // Save the NutanixWorkerNodeConfigSpecByMachineDeploymentName only if it contains at least one Nutanix configuration. + if len(nutanixWorkerNodeConfigSpecByMachineDeploymentName) > 0 { + n.nutanixWorkerNodeConfigSpecByMachineDeploymentName = nutanixWorkerNodeConfigSpecByMachineDeploymentName + } + + return func(ctx context.Context) preflight.CheckResult { + return result + } +} From da8abb621bf12c024a7e4591d5459628f94444c6 Mon Sep 17 00:00:00 2001 From: Daniel Lipovetsky Date: Wed, 4 Jun 2025 11:59:47 -0700 Subject: [PATCH 02/12] fixup! feat: Nutanix VM image preflight check Allow imageLookup with warning --- pkg/webhook/preflight/nutanix/image.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/pkg/webhook/preflight/nutanix/image.go b/pkg/webhook/preflight/nutanix/image.go index 4af21a44f..03dce9c25 100644 --- a/pkg/webhook/preflight/nutanix/image.go +++ b/pkg/webhook/preflight/nutanix/image.go @@ -73,12 +73,11 @@ func (n *nutanixChecker) vmImageCheck( } if machineDetails.ImageLookup != nil { - result.Allowed = false - result.Error = true - result.Causes = append(result.Causes, preflight.Cause{ - Message: "ImageLookup is not yet supported", - Field: field, - }) + result.Allowed = true + result.Warnings = append( + result.Warnings, + fmt.Sprintf("%s uses imageLookup, which is not yet supported by checks", field), + ) return result } From 0eaea5a2c25288886b0fae5d018a8f987eef39bf Mon Sep 17 00:00:00 2001 From: Daniel Lipovetsky Date: Tue, 3 Jun 2025 13:56:06 -0700 Subject: [PATCH 03/12] fixup! feat: Nutanix VM image preflight check Mock nutanix clients to allow unit tests --- pkg/webhook/preflight/nutanix/checker.go | 13 ++- pkg/webhook/preflight/nutanix/clients.go | 94 ++++++++++++++++++++ pkg/webhook/preflight/nutanix/credentials.go | 8 +- pkg/webhook/preflight/nutanix/image.go | 8 +- 4 files changed, 109 insertions(+), 14 deletions(-) create mode 100644 pkg/webhook/preflight/nutanix/clients.go diff --git a/pkg/webhook/preflight/nutanix/checker.go b/pkg/webhook/preflight/nutanix/checker.go index c6c7a7f8a..a348cd718 100644 --- a/pkg/webhook/preflight/nutanix/checker.go +++ b/pkg/webhook/preflight/nutanix/checker.go @@ -12,8 +12,6 @@ import ( ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" prismgoclient "github.com/nutanix-cloud-native/prism-go-client" - prismv3 "github.com/nutanix-cloud-native/prism-go-client/v3" - prismv4 "github.com/nutanix-cloud-native/prism-go-client/v4" carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight" @@ -23,6 +21,9 @@ func New(kclient ctrlclient.Client, cluster *clusterv1.Cluster) preflight.Checke return &nutanixChecker{ kclient: kclient, cluster: cluster, + + v3clientFactory: newV3Client, + v4clientFactory: newV4Client, } } @@ -34,8 +35,12 @@ type nutanixChecker struct { nutanixWorkerNodeConfigSpecByMachineDeploymentName map[string]*carenv1.NutanixWorkerNodeConfigSpec credentials prismgoclient.Credentials - v3client *prismv3.Client - v4client *prismv4.Client + + v3client v3client + v3clientFactory func(prismgoclient.Credentials) (v3client, error) + + v4client v4client + v4clientFactory func(prismgoclient.Credentials) (v4client, error) log logr.Logger } diff --git a/pkg/webhook/preflight/nutanix/clients.go b/pkg/webhook/preflight/nutanix/clients.go new file mode 100644 index 000000000..874d64891 --- /dev/null +++ b/pkg/webhook/preflight/nutanix/clients.go @@ -0,0 +1,94 @@ +package nutanix + +import ( + "context" + + vmmv4 "github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4/models/vmm/v4/content" + + prismgoclient "github.com/nutanix-cloud-native/prism-go-client" + prismv3 "github.com/nutanix-cloud-native/prism-go-client/v3" + prismv4 "github.com/nutanix-cloud-native/prism-go-client/v4" +) + +type v3client interface { + GetCurrentLoggedInUser(ctx context.Context) (*prismv3.UserIntentResponse, error) +} + +type v3clientWrapper struct { + prismv3.Service +} + +var _ = v3client(&v3clientWrapper{}) + +func newV3Client( + credentials prismgoclient.Credentials, //nolint:gocritic // hugeParam is fine +) (v3client, error) { + client, err := prismv3.NewV3Client(credentials) + if err != nil { + return nil, err + } + return &v3clientWrapper{ + client.V3, + }, nil +} + +type v4client interface { + GetImageById(id *string) (*vmmv4.GetImageApiResponse, error) + ListImages(page_ *int, + limit_ *int, + filter_ *string, + orderby_ *string, + select_ *string, + args ...map[string]interface{}, + ) ( + *vmmv4.ListImagesApiResponse, + error, + ) +} + +type v4clientWrapper struct { + client *prismv4.Client +} + +var _ = v4client(&v4clientWrapper{}) + +func (c *v4clientWrapper) GetImageById(id *string) (*vmmv4.GetImageApiResponse, error) { + resp, err := c.client.ImagesApiInstance.GetImageById(id) + if err != nil { + return nil, err + } + return resp, nil +} + +func (c *v4clientWrapper) ListImages(page_ *int, + limit_ *int, + filter_ *string, + orderby_ *string, + select_ *string, + args ...map[string]interface{}, +) (*vmmv4.ListImagesApiResponse, error) { + resp, err := c.client.ImagesApiInstance.ListImages( + page_, + limit_, + filter_, + orderby_, + select_, + args..., + ) + if err != nil { + return nil, err + } + return resp, nil +} + +func newV4Client( + credentials prismgoclient.Credentials, //nolint:gocritic // hugeParam is fine +) (v4client, error) { + client, err := prismv4.NewV4Client(credentials) + if err != nil { + return nil, err + } + return &v4clientWrapper{ + client: client, + }, nil +} diff --git a/pkg/webhook/preflight/nutanix/credentials.go b/pkg/webhook/preflight/nutanix/credentials.go index 2464ad143..b8f54fcf2 100644 --- a/pkg/webhook/preflight/nutanix/credentials.go +++ b/pkg/webhook/preflight/nutanix/credentials.go @@ -12,8 +12,6 @@ import ( prismgoclient "github.com/nutanix-cloud-native/prism-go-client" prismcredentials "github.com/nutanix-cloud-native/prism-go-client/environment/credentials" - prismv3 "github.com/nutanix-cloud-native/prism-go-client/v3" - prismv4 "github.com/nutanix-cloud-native/prism-go-client/v4" "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight" ) @@ -164,7 +162,7 @@ func (n *nutanixChecker) initCredentialsCheck(ctx context.Context) preflight.Che } // Initialize the clients. - n.v4client, err = prismv4.NewV4Client(n.credentials) + n.v4client, err = n.v4clientFactory(n.credentials) if err != nil { result.Allowed = false result.Error = true @@ -176,7 +174,7 @@ func (n *nutanixChecker) initCredentialsCheck(ctx context.Context) preflight.Che ) } - n.v3client, err = prismv3.NewV3Client(n.credentials) + n.v3client, err = n.v3clientFactory(n.credentials) if err != nil { result.Allowed = false result.Error = true @@ -187,7 +185,7 @@ func (n *nutanixChecker) initCredentialsCheck(ctx context.Context) preflight.Che }, ) } - _, err = n.v3client.V3.GetCurrentLoggedInUser(ctx) + _, err = n.v3client.GetCurrentLoggedInUser(ctx) if err != nil { result.Allowed = false result.Error = true diff --git a/pkg/webhook/preflight/nutanix/image.go b/pkg/webhook/preflight/nutanix/image.go index 03dce9c25..28397219d 100644 --- a/pkg/webhook/preflight/nutanix/image.go +++ b/pkg/webhook/preflight/nutanix/image.go @@ -9,8 +9,6 @@ import ( vmmv4 "github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4/models/vmm/v4/content" - prismv4 "github.com/nutanix-cloud-native/prism-go-client/v4" - capxv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/external/github.com/nutanix-cloud-native/cluster-api-provider-nutanix/api/v1beta1" carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight" @@ -118,12 +116,12 @@ func (n *nutanixChecker) vmImageCheck( } func getVMImages( - client *prismv4.Client, + client v4client, id *capxv1.NutanixResourceIdentifier, ) ([]vmmv4.Image, error) { switch { case id.IsUUID(): - resp, err := client.ImagesApiInstance.GetImageById(id.UUID) + resp, err := client.GetImageById(id.UUID) if err != nil { return nil, err } @@ -134,7 +132,7 @@ func getVMImages( return []vmmv4.Image{image}, nil case id.IsName(): filter_ := fmt.Sprintf("name eq '%s'", *id.Name) - resp, err := client.ImagesApiInstance.ListImages(nil, nil, &filter_, nil, nil) + resp, err := client.ListImages(nil, nil, &filter_, nil, nil) if err != nil { return nil, err } From 45b4b86ea1670c420aacb1221c259d966f6fdf0e Mon Sep 17 00:00:00 2001 From: Daniel Lipovetsky Date: Tue, 3 Jun 2025 16:16:55 -0700 Subject: [PATCH 04/12] fixup! feat: Nutanix VM image preflight check * Refactor nutanixChecker to allow unit tests of all checks. * Add unit tests. * Fix field reported when spec init for worker spec fails. * Fix handling of credentials "Insecure" option. * Use lowercase 'v' in errors that refers to the Nutanix client --- pkg/webhook/preflight/nutanix/checker.go | 30 +- pkg/webhook/preflight/nutanix/checker_test.go | 154 ++++ pkg/webhook/preflight/nutanix/clients.go | 3 + pkg/webhook/preflight/nutanix/credentials.go | 45 +- .../preflight/nutanix/credentials_test.go | 261 +++++++ pkg/webhook/preflight/nutanix/image.go | 13 +- pkg/webhook/preflight/nutanix/image_test.go | 710 ++++++++++++++++++ pkg/webhook/preflight/nutanix/specs.go | 10 +- pkg/webhook/preflight/nutanix/specs_test.go | 387 ++++++++++ 9 files changed, 1579 insertions(+), 34 deletions(-) create mode 100644 pkg/webhook/preflight/nutanix/checker_test.go create mode 100644 pkg/webhook/preflight/nutanix/credentials_test.go create mode 100644 pkg/webhook/preflight/nutanix/image_test.go create mode 100644 pkg/webhook/preflight/nutanix/specs_test.go diff --git a/pkg/webhook/preflight/nutanix/checker.go b/pkg/webhook/preflight/nutanix/checker.go index a348cd718..8fb047191 100644 --- a/pkg/webhook/preflight/nutanix/checker.go +++ b/pkg/webhook/preflight/nutanix/checker.go @@ -24,6 +24,11 @@ func New(kclient ctrlclient.Client, cluster *clusterv1.Cluster) preflight.Checke v3clientFactory: newV3Client, v4clientFactory: newV4Client, + + vmImageCheckFunc: vmImageCheck, + initNutanixConfigurationFunc: initNutanixConfiguration, + initCredentialsCheckFunc: initCredentialsCheck, + initVMImageChecksFunc: initVMImageChecks, } } @@ -42,6 +47,25 @@ type nutanixChecker struct { v4client v4client v4clientFactory func(prismgoclient.Credentials) (v4client, error) + vmImageCheckFunc func( + n *nutanixChecker, + machineDetails *carenv1.NutanixMachineDetails, + field string, + ) preflight.Check + + initNutanixConfigurationFunc func( + n *nutanixChecker, + ) preflight.Check + + initCredentialsCheckFunc func( + ctx context.Context, + n *nutanixChecker, + ) preflight.Check + + initVMImageChecksFunc func( + n *nutanixChecker, + ) []preflight.Check + log logr.Logger } @@ -53,11 +77,11 @@ func (n *nutanixChecker) Init( checks := []preflight.Check{ // The configuration check must run first, because it initializes data used by all other checks, // and the credentials check second, because it initializes the Nutanix clients used by other checks. - n.initNutanixConfiguration(), - n.initCredentialsCheck(ctx), + n.initNutanixConfigurationFunc(n), + n.initCredentialsCheckFunc(ctx, n), } - checks = append(checks, n.initVMImageChecks()...) + checks = append(checks, n.initVMImageChecksFunc(n)...) // Add more checks here as needed. diff --git a/pkg/webhook/preflight/nutanix/checker_test.go b/pkg/webhook/preflight/nutanix/checker_test.go new file mode 100644 index 000000000..93f054207 --- /dev/null +++ b/pkg/webhook/preflight/nutanix/checker_test.go @@ -0,0 +1,154 @@ +// Copyright 2024 Nutanix. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package nutanix + +import ( + "context" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + + carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" + "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight" +) + +func TestNutanixChecker_Init(t *testing.T) { + tests := []struct { + name string + nutanixConfig *carenv1.NutanixClusterConfigSpec + workerNodeConfigs map[string]*carenv1.NutanixWorkerNodeConfigSpec + expectedCheckCount int + expectedFirstCheckName string + expectedSecondCheckName string + vmImageCheckCount int + }{ + { + name: "basic initialization with no configs", + nutanixConfig: nil, + workerNodeConfigs: nil, + expectedCheckCount: 2, // config check and credentials check + expectedFirstCheckName: "NutanixConfiguration", + expectedSecondCheckName: "NutanixCredentials", + vmImageCheckCount: 0, + }, + { + name: "initialization with control plane config", + nutanixConfig: &carenv1.NutanixClusterConfigSpec{ + ControlPlane: &carenv1.NutanixControlPlaneSpec{ + Nutanix: &carenv1.NutanixNodeSpec{}, + }, + }, + workerNodeConfigs: nil, + expectedCheckCount: 3, // config check, credentials check, 1 VM image check + expectedFirstCheckName: "NutanixConfiguration", + expectedSecondCheckName: "NutanixCredentials", + vmImageCheckCount: 1, + }, + { + name: "initialization with worker node configs", + nutanixConfig: nil, + workerNodeConfigs: map[string]*carenv1.NutanixWorkerNodeConfigSpec{ + "worker-1": { + Nutanix: &carenv1.NutanixNodeSpec{}, + }, + "worker-2": { + Nutanix: &carenv1.NutanixNodeSpec{}, + }, + }, + expectedCheckCount: 4, // config check, credentials check, 2 VM image checks + expectedFirstCheckName: "NutanixConfiguration", + expectedSecondCheckName: "NutanixCredentials", + vmImageCheckCount: 2, + }, + { + name: "initialization with both control plane and worker node configs", + nutanixConfig: &carenv1.NutanixClusterConfigSpec{ + ControlPlane: &carenv1.NutanixControlPlaneSpec{ + Nutanix: &carenv1.NutanixNodeSpec{}, + }, + }, + workerNodeConfigs: map[string]*carenv1.NutanixWorkerNodeConfigSpec{ + "worker-1": { + Nutanix: &carenv1.NutanixNodeSpec{}, + }, + }, + expectedCheckCount: 4, // config check, credentials check, 2 VM image checks (1 CP + 1 worker) + expectedFirstCheckName: "NutanixConfiguration", + expectedSecondCheckName: "NutanixCredentials", + vmImageCheckCount: 2, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create the checker + checker := &nutanixChecker{ + cluster: &clusterv1.Cluster{}, + nutanixClusterConfigSpec: tt.nutanixConfig, + nutanixWorkerNodeConfigSpecByMachineDeploymentName: tt.workerNodeConfigs, + } + + // Mock the sub-check functions to track their calls + configCheckCalled := false + credsCheckCalled := false + vmImageCheckCount := 0 + + checker.initNutanixConfigurationFunc = func(n *nutanixChecker) preflight.Check { + configCheckCalled = true + return func(ctx context.Context) preflight.CheckResult { + return preflight.CheckResult{ + Name: tt.expectedFirstCheckName, + } + } + } + + checker.initCredentialsCheckFunc = func(ctx context.Context, n *nutanixChecker) preflight.Check { + credsCheckCalled = true + return func(ctx context.Context) preflight.CheckResult { + return preflight.CheckResult{ + Name: tt.expectedSecondCheckName, + } + } + } + + checker.initVMImageChecksFunc = func(n *nutanixChecker) []preflight.Check { + checks := []preflight.Check{} + for i := 0; i < tt.vmImageCheckCount; i++ { + vmImageCheckCount++ + checks = append(checks, func(ctx context.Context) preflight.CheckResult { + return preflight.CheckResult{ + Name: fmt.Sprintf("VMImageCheck-%d", i), + } + }) + } + return checks + } + + // Call Init + ctx := context.Background() + checks := checker.Init(ctx) + + // Verify the logger was set + assert.NotNil(t, checker.log) + + // Verify correct number of checks + assert.Len(t, checks, tt.expectedCheckCount) + + // Verify the sub-functions were called + assert.True(t, configCheckCalled, "initNutanixConfiguration should have been called") + assert.True(t, credsCheckCalled, "initCredentialsCheck should have been called") + assert.Equal(t, tt.vmImageCheckCount, vmImageCheckCount, "Wrong number of VM image checks") + + // Verify the first two checks when we have results + if len(checks) >= 2 { + result1 := checks[0](ctx) + result2 := checks[1](ctx) + assert.Equal(t, tt.expectedFirstCheckName, result1.Name) + assert.Equal(t, tt.expectedSecondCheckName, result2.Name) + } + }) + } +} diff --git a/pkg/webhook/preflight/nutanix/clients.go b/pkg/webhook/preflight/nutanix/clients.go index 874d64891..cac4a29a1 100644 --- a/pkg/webhook/preflight/nutanix/clients.go +++ b/pkg/webhook/preflight/nutanix/clients.go @@ -1,3 +1,6 @@ +// Copyright 2024 Nutanix. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + package nutanix import ( diff --git a/pkg/webhook/preflight/nutanix/credentials.go b/pkg/webhook/preflight/nutanix/credentials.go index b8f54fcf2..31b616799 100644 --- a/pkg/webhook/preflight/nutanix/credentials.go +++ b/pkg/webhook/preflight/nutanix/credentials.go @@ -18,7 +18,10 @@ import ( const credentialsSecretDataKey = "credentials" -func (n *nutanixChecker) initCredentialsCheck(ctx context.Context) preflight.Check { +func initCredentialsCheck( + ctx context.Context, + n *nutanixChecker, +) preflight.Check { n.log.V(5).Info("Initializing Nutanix credentials check") result := preflight.CheckResult{ @@ -142,23 +145,13 @@ func (n *nutanixChecker) initCredentialsCheck(ctx context.Context) preflight.Che } } - if !result.Allowed || result.Error { - // If any error has happened, we should not try to initialize the credentials or clients, so we return early. - return func(ctx context.Context) preflight.CheckResult { - return result - } - } - // Initialize the credentials. n.credentials = prismgoclient.Credentials{ Endpoint: fmt.Sprintf("%s:%d", host, port), URL: fmt.Sprintf("https://%s:%d", host, port), Username: usernamePassword.Username, Password: usernamePassword.Password, - } - if prismCentralEndpointSpec.Insecure { - n.credentials.Insecure = true - n.credentials.URL = fmt.Sprintf("http://%s:%d", host, port) + Insecure: prismCentralEndpointSpec.Insecure, } // Initialize the clients. @@ -168,7 +161,7 @@ func (n *nutanixChecker) initCredentialsCheck(ctx context.Context) preflight.Che result.Error = true result.Causes = append(result.Causes, preflight.Cause{ - Message: fmt.Sprintf("failed to initialize Nutanix V4 client: %s", err), + Message: fmt.Sprintf("failed to initialize Nutanix v4 client: %s", err), Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials", }, ) @@ -180,21 +173,23 @@ func (n *nutanixChecker) initCredentialsCheck(ctx context.Context) preflight.Che result.Error = true result.Causes = append(result.Causes, preflight.Cause{ - Message: fmt.Sprintf("failed to initialize Nutanix V3 client: %s", err), - Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials", - }, - ) - } - _, err = n.v3client.GetCurrentLoggedInUser(ctx) - if err != nil { - result.Allowed = false - result.Error = true - result.Causes = append(result.Causes, - preflight.Cause{ - Message: fmt.Sprintf("failed to validate credentials using the v3 API client: %s", err), + Message: fmt.Sprintf("failed to initialize Nutanix v3 client: %s", err), Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials", }, ) + } else { + // Validate the credentials using an API call. + _, err = n.v3client.GetCurrentLoggedInUser(ctx) + if err != nil { + result.Allowed = false + result.Error = true + result.Causes = append(result.Causes, + preflight.Cause{ + Message: fmt.Sprintf("failed to validate credentials using the v3 API client: %s", err), + Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials", + }, + ) + } } return func(ctx context.Context) preflight.CheckResult { diff --git a/pkg/webhook/preflight/nutanix/credentials_test.go b/pkg/webhook/preflight/nutanix/credentials_test.go new file mode 100644 index 000000000..40ad68426 --- /dev/null +++ b/pkg/webhook/preflight/nutanix/credentials_test.go @@ -0,0 +1,261 @@ +// Copyright 2024 Nutanix. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package nutanix + +import ( + "context" + "testing" + + vmmv4 "github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4/models/vmm/v4/content" + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + prismgoclient "github.com/nutanix-cloud-native/prism-go-client" + prismv3 "github.com/nutanix-cloud-native/prism-go-client/v3" + + carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" +) + +func TestInitCredentialsCheck_Success(t *testing.T) { + nc := validNutanixChecker() + check := nc.initCredentialsCheckFunc(context.Background(), nc) + result := check(context.Background()) + assert.True(t, result.Allowed) + assert.False(t, result.Error) + assert.Empty(t, result.Causes) +} + +func TestInitCredentialsCheck_NoNutanixConfig(t *testing.T) { + nc := validNutanixChecker() + nc.nutanixClusterConfigSpec = nil + nc.nutanixWorkerNodeConfigSpecByMachineDeploymentName = map[string]*carenv1.NutanixWorkerNodeConfigSpec{} + check := nc.initCredentialsCheckFunc(context.Background(), nc) + result := check(context.Background()) + assert.True(t, result.Allowed) + assert.False(t, result.Error) + assert.Empty(t, result.Causes) +} + +func TestInitCredentialsCheck_MissingNutanixField(t *testing.T) { + nc := validNutanixChecker() + nc.nutanixClusterConfigSpec.Nutanix = nil + check := nc.initCredentialsCheckFunc(context.Background(), nc) + result := check(context.Background()) + assert.False(t, result.Allowed) + assert.True(t, result.Error) + assert.NotEmpty(t, result.Causes) + assert.Contains(t, result.Causes[0].Message, "Nutanix cluster configuration is not defined") +} + +func TestInitCredentialsCheck_InvalidURL(t *testing.T) { + nc := validNutanixChecker() + nc.nutanixClusterConfigSpec.Nutanix.PrismCentralEndpoint.URL = "not-a-url" + check := nc.initCredentialsCheckFunc(context.Background(), nc) + result := check(context.Background()) + assert.False(t, result.Allowed) + assert.True(t, result.Error) + assert.Contains(t, result.Causes[0].Message, "failed to parse Prism Central endpoint URL") +} + +func TestInitCredentialsCheck_SecretNotFound(t *testing.T) { + nc := validNutanixChecker() + nc.kclient = fake.NewClientBuilder().Build() // no secret + check := nc.initCredentialsCheckFunc(context.Background(), nc) + result := check(context.Background()) + assert.False(t, result.Allowed) + assert.True(t, result.Error) + assert.Contains(t, result.Causes[0].Message, "failed to get Prism Central credentials Secret") +} + +func TestInitCredentialsCheck_SecretEmpty(t *testing.T) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ntnx-creds", + Namespace: "default", + }, + Data: map[string][]byte{}, + } + kclient := fake.NewClientBuilder().WithObjects(secret).Build() + nc := validNutanixChecker() + nc.kclient = kclient + check := nc.initCredentialsCheckFunc(context.Background(), nc) + result := check(context.Background()) + assert.False(t, result.Allowed) + assert.True(t, result.Error) + assert.Contains(t, result.Causes[0].Message, "credentials Secret 'ntnx-creds' is empty") +} + +func TestInitCredentialsCheck_SecretMissingKey(t *testing.T) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ntnx-creds", + Namespace: "default", + }, + Data: map[string][]byte{ + "not-credentials": []byte("foo"), + }, + } + kclient := fake.NewClientBuilder().WithObjects(secret).Build() + nc := validNutanixChecker() + nc.kclient = kclient + check := nc.initCredentialsCheckFunc(context.Background(), nc) + result := check(context.Background()) + assert.False(t, result.Allowed) + assert.True(t, result.Error) + assert.Contains(t, result.Causes[0].Message, "does not contain key 'credentials'") +} + +func TestInitCredentialsCheck_InvalidCredentialsFormat(t *testing.T) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ntnx-creds", + Namespace: "default", + }, + Data: map[string][]byte{ + "credentials": []byte("not-a-valid-format"), + }, + } + kclient := fake.NewClientBuilder().WithObjects(secret).Build() + nc := validNutanixChecker() + nc.kclient = kclient + check := nc.initCredentialsCheckFunc(context.Background(), nc) + result := check(context.Background()) + assert.False(t, result.Allowed) + assert.True(t, result.Error) + assert.Contains(t, result.Causes[0].Message, "failed to parse Prism Central credentials") +} + +func TestInitCredentialsCheck_FailedToCreateV3Client(t *testing.T) { + // Simulate a failure in creating the v3 client + nc := validNutanixChecker() + nc.v3clientFactory = func(_ prismgoclient.Credentials) (v3client, error) { + return nil, assert.AnError + } + nc.v4clientFactory = func(_ prismgoclient.Credentials) (v4client, error) { + return &mockv4client{}, nil + } + check := nc.initCredentialsCheckFunc(context.Background(), nc) + result := check(context.Background()) + assert.False(t, result.Allowed) + assert.True(t, result.Error) + assert.Contains(t, result.Causes[0].Message, "failed to initialize Nutanix v3 client") +} + +func TestInitCredentialsCheck_FailedToCreateV4Client(t *testing.T) { + // Simulate a failure in creating the v4 client + nc := validNutanixChecker() + nc.v3clientFactory = func(_ prismgoclient.Credentials) (v3client, error) { + return &mockv3client{}, nil + } + nc.v4clientFactory = func(_ prismgoclient.Credentials) (v4client, error) { + return nil, assert.AnError + } + check := nc.initCredentialsCheckFunc(context.Background(), nc) + result := check(context.Background()) + assert.False(t, result.Allowed) + assert.True(t, result.Error) + assert.Contains(t, result.Causes[0].Message, "failed to initialize Nutanix v4 client") +} + +func TestInitCredentialsCheck_FailedToGetCurrentLoggedInUser(t *testing.T) { + // Simulate a failure in getting the current logged-in user + nc := validNutanixChecker() + nc.v3clientFactory = func(_ prismgoclient.Credentials) (v3client, error) { + return &mockv3client{err: assert.AnError}, nil + } + nc.v4clientFactory = func(_ prismgoclient.Credentials) (v4client, error) { + return &mockv4client{}, nil + } + check := nc.initCredentialsCheckFunc(context.Background(), nc) + result := check(context.Background()) + assert.False(t, result.Allowed) + assert.True(t, result.Error) + assert.Contains(t, result.Causes[0].Message, "failed to validate credentials using the v3 API client") +} + +func validNutanixChecker() *nutanixChecker { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ntnx-creds", + Namespace: "default", + }, + Data: map[string][]byte{ + "credentials": []byte(`[ + { + "type": "basic_auth", + "data": { + "prismCentral": { + "username": "testuser", + "password": "testpassword" + } + } + } + ]`), + }, + } + kclient := fake.NewClientBuilder().WithObjects(secret).Build() + return &nutanixChecker{ + kclient: kclient, + cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + }, + + v3clientFactory: func(_ prismgoclient.Credentials) (v3client, error) { + return &mockv3client{}, nil + }, + v4clientFactory: func(_ prismgoclient.Credentials) (v4client, error) { + return &mockv4client{}, nil + }, + + vmImageCheckFunc: vmImageCheck, + initNutanixConfigurationFunc: initNutanixConfiguration, + initCredentialsCheckFunc: initCredentialsCheck, + + nutanixClusterConfigSpec: &carenv1.NutanixClusterConfigSpec{ + Nutanix: &carenv1.NutanixSpec{ + PrismCentralEndpoint: carenv1.NutanixPrismCentralEndpointSpec{ + URL: "https://pc.example.com:9440", + Credentials: carenv1.NutanixPrismCentralEndpointCredentials{ + SecretRef: carenv1.LocalObjectReference{ + Name: "ntnx-creds", + }, + }, + }, + }, + }, + nutanixWorkerNodeConfigSpecByMachineDeploymentName: map[string]*carenv1.NutanixWorkerNodeConfigSpec{}, + } +} + +type mockv3client struct { + user *prismv3.UserIntentResponse + err error +} + +func (m *mockv3client) GetCurrentLoggedInUser(ctx context.Context) (*prismv3.UserIntentResponse, error) { + return m.user, m.err +} + +type mockv4client struct { + image *vmmv4.GetImageApiResponse + images *vmmv4.ListImagesApiResponse +} + +func (m *mockv4client) GetImageById(id *string) (*vmmv4.GetImageApiResponse, error) { + return m.image, nil +} + +func (m *mockv4client) ListImages( + _, _ *int, + _, _, _ *string, + _ ...map[string]interface{}, +) (*vmmv4.ListImagesApiResponse, error) { + return m.images, nil +} diff --git a/pkg/webhook/preflight/nutanix/image.go b/pkg/webhook/preflight/nutanix/image.go index 28397219d..b2c9542c6 100644 --- a/pkg/webhook/preflight/nutanix/image.go +++ b/pkg/webhook/preflight/nutanix/image.go @@ -14,13 +14,16 @@ import ( "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight" ) -func (n *nutanixChecker) initVMImageChecks() []preflight.Check { +func initVMImageChecks( + n *nutanixChecker, +) []preflight.Check { checks := []preflight.Check{} if n.nutanixClusterConfigSpec != nil && n.nutanixClusterConfigSpec.ControlPlane != nil && n.nutanixClusterConfigSpec.ControlPlane.Nutanix != nil { checks = append(checks, - n.vmImageCheck( + n.vmImageCheckFunc( + n, &n.nutanixClusterConfigSpec.ControlPlane.Nutanix.MachineDetails, "cluster.spec.topology[.name=clusterConfig].value.controlPlane.nutanix.machineDetails", ), @@ -30,7 +33,8 @@ func (n *nutanixChecker) initVMImageChecks() []preflight.Check { for mdName, nutanixWorkerNodeConfigSpec := range n.nutanixWorkerNodeConfigSpecByMachineDeploymentName { if nutanixWorkerNodeConfigSpec.Nutanix != nil { checks = append(checks, - n.vmImageCheck( + n.vmImageCheckFunc( + n, &nutanixWorkerNodeConfigSpec.Nutanix.MachineDetails, fmt.Sprintf( "cluster.spec.topology.workers.machineDeployments[.name=%s]"+ @@ -45,7 +49,8 @@ func (n *nutanixChecker) initVMImageChecks() []preflight.Check { return checks } -func (n *nutanixChecker) vmImageCheck( +func vmImageCheck( + n *nutanixChecker, machineDetails *carenv1.NutanixMachineDetails, field string, ) preflight.Check { diff --git a/pkg/webhook/preflight/nutanix/image_test.go b/pkg/webhook/preflight/nutanix/image_test.go new file mode 100644 index 000000000..ce771f9a3 --- /dev/null +++ b/pkg/webhook/preflight/nutanix/image_test.go @@ -0,0 +1,710 @@ +// Copyright 2025 Nutanix. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package nutanix + +import ( + "context" + "fmt" + "testing" + + "github.com/go-logr/logr/testr" + vmmv4 "github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4/models/vmm/v4/content" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/utils/ptr" + + capxv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/external/github.com/nutanix-cloud-native/cluster-api-provider-nutanix/api/v1beta1" + carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" + "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight" +) + +// mockV4Client is a mock implementation of the v4client interface for testing. +type mockV4Client struct { + getImageByIdFunc func( + uuid *string, + ) ( + *vmmv4.GetImageApiResponse, error, + ) + + listImagesFunc func( + page, + limit *int, + filter, + orderby, + select_ *string, + args ...map[string]interface{}, + ) ( + *vmmv4.ListImagesApiResponse, + error, + ) +} + +func (m *mockV4Client) GetImageById(uuid *string) (*vmmv4.GetImageApiResponse, error) { + return m.getImageByIdFunc(uuid) +} + +func (m *mockV4Client) ListImages( + page, limit *int, + filter, orderby, select_ *string, + args ...map[string]interface{}, +) (*vmmv4.ListImagesApiResponse, error) { + return m.listImagesFunc(page, limit, filter, orderby, select_) +} + +func TestVMImageCheck(t *testing.T) { + testCases := []struct { + name string + v4client v4client + machineDetails *carenv1.NutanixMachineDetails + want preflight.CheckResult + }{ + { + name: "Nutanix v4 client not initialized", + v4client: nil, + machineDetails: &carenv1.NutanixMachineDetails{ + Image: &capxv1.NutanixResourceIdentifier{ + Type: capxv1.NutanixIdentifierUUID, + UUID: ptr.To("test-uuid"), + }, + }, + want: preflight.CheckResult{ + Name: "NutanixVMImage", + Allowed: false, + Error: true, + Causes: []preflight.Cause{ + { + Message: "Nutanix v4 client is not initialized, cannot perform VM image checks", + Field: "", + }, + }, + }, + }, + { + name: "imageLookup not yet supported", + v4client: &mockV4Client{}, + machineDetails: &carenv1.NutanixMachineDetails{ + ImageLookup: &capxv1.NutanixImageLookup{ + Format: ptr.To("test-format"), + BaseOS: "test-baseos", + }, + }, + want: preflight.CheckResult{ + Name: "NutanixVMImage", + Allowed: true, + Warnings: []string{ + "test-field uses imageLookup, which is not yet supported by checks", + }, + }, + }, + { + name: "image found by uuid", + v4client: &mockV4Client{ + getImageByIdFunc: func(uuid *string) (*vmmv4.GetImageApiResponse, error) { + assert.Equal(t, "test-uuid", *uuid) + resp := &vmmv4.GetImageApiResponse{} + err := resp.SetData(vmmv4.Image{ + ObjectType_: ptr.To("vmm.v4.content.Image"), + ExtId: ptr.To("test-uuid"), + }) + require.NoError(t, err) + return resp, nil + }, + }, + machineDetails: &carenv1.NutanixMachineDetails{ + Image: &capxv1.NutanixResourceIdentifier{ + Type: capxv1.NutanixIdentifierUUID, + UUID: ptr.To("test-uuid"), + }, + }, + want: preflight.CheckResult{ + Name: "NutanixVMImage", + Allowed: true, + }, + }, + { + name: "image found by name", + v4client: &mockV4Client{ + listImagesFunc: func(page, + limit *int, + filter, + orderby, + select_ *string, + args ...map[string]interface{}, + ) ( + *vmmv4.ListImagesApiResponse, + error, + ) { + resp := &vmmv4.ListImagesApiResponse{} + err := resp.SetData([]vmmv4.Image{ + { + Name: ptr.To("test-image-name"), + }, + }) + require.NoError(t, err) + return resp, nil + }, + }, + machineDetails: &carenv1.NutanixMachineDetails{ + Image: &capxv1.NutanixResourceIdentifier{ + Type: capxv1.NutanixIdentifierName, + Name: ptr.To("test-image-name"), + }, + }, + want: preflight.CheckResult{ + Name: "NutanixVMImage", + Allowed: true, + }, + }, + { + name: "image not found by name", + v4client: &mockV4Client{ + listImagesFunc: func(page, + limit *int, + filter, + orderby, + select_ *string, + args ...map[string]interface{}, + ) ( + *vmmv4.ListImagesApiResponse, + error, + ) { + return &vmmv4.ListImagesApiResponse{}, nil + }, + }, + machineDetails: &carenv1.NutanixMachineDetails{ + Image: &capxv1.NutanixResourceIdentifier{ + Type: capxv1.NutanixIdentifierName, + Name: ptr.To("test-non-existent-image"), + }, + }, + want: preflight.CheckResult{ + Name: "NutanixVMImage", + Allowed: false, + Causes: []preflight.Cause{ + { + Message: "expected to find 1 VM Image, found 0", + Field: "test-field", + }, + }, + }, + }, + { + name: "multiple images found by name", + v4client: &mockV4Client{ + listImagesFunc: func(page, + limit *int, + filter, + orderby, + select_ *string, + args ...map[string]interface{}, + ) ( + *vmmv4.ListImagesApiResponse, + error, + ) { + resp := &vmmv4.ListImagesApiResponse{} + err := resp.SetData([]vmmv4.Image{ + { + Name: ptr.To("test-duplicate-image"), + }, + { + Name: ptr.To("test-duplicate-image"), + }, + }) + require.NoError(t, err) + return resp, nil + }, + }, + machineDetails: &carenv1.NutanixMachineDetails{ + Image: &capxv1.NutanixResourceIdentifier{ + Type: capxv1.NutanixIdentifierName, + Name: ptr.To("test-duplicate-image"), + }, + }, + want: preflight.CheckResult{ + Name: "NutanixVMImage", + Allowed: false, + Causes: []preflight.Cause{ + { + Message: "expected to find 1 VM Image, found 2", + Field: "test-field", + }, + }, + }, + }, + { + name: "error getting image by id", + v4client: &mockV4Client{ + getImageByIdFunc: func(uuid *string) (*vmmv4.GetImageApiResponse, error) { + return nil, fmt.Errorf("api error") + }, + }, + machineDetails: &carenv1.NutanixMachineDetails{ + Image: &capxv1.NutanixResourceIdentifier{ + Type: capxv1.NutanixIdentifierUUID, + UUID: ptr.To("test-uuid"), + }, + }, + want: preflight.CheckResult{ + Name: "NutanixVMImage", + Allowed: false, + Error: true, + Causes: []preflight.Cause{ + { + Message: "failed to get VM Image: api error", + Field: "test-field", + }, + }, + }, + }, + { + name: "error listing images", + v4client: &mockV4Client{ + listImagesFunc: func(page, + limit *int, + filter, + orderby, + select_ *string, + args ...map[string]interface{}, + ) ( + *vmmv4.ListImagesApiResponse, + error, + ) { + return nil, fmt.Errorf("api error") + }, + }, + machineDetails: &carenv1.NutanixMachineDetails{ + Image: &capxv1.NutanixResourceIdentifier{ + Type: capxv1.NutanixIdentifierName, + Name: ptr.To("test-image"), + }, + }, + want: preflight.CheckResult{ + Name: "NutanixVMImage", + Allowed: false, + Error: true, + Causes: []preflight.Cause{ + { + Message: "failed to get VM Image: api error", + Field: "test-field", + }, + }, + }, + }, + { + name: "neither image nor imageLookup specified", + v4client: &mockV4Client{}, + machineDetails: &carenv1.NutanixMachineDetails{ + // both Image and ImageLookup are nil + }, + want: preflight.CheckResult{ + Name: "NutanixVMImage", + Allowed: false, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + logger := testr.New(t) + + checker := &nutanixChecker{ + log: logger, + v4client: tc.v4client, + } + + // Create the check + checkFn := vmImageCheck( + checker, + tc.machineDetails, + "test-field", + ) + + // Execute the check + got := checkFn(context.Background()) + + // Verify the result + assert.Equal(t, tc.want.Name, got.Name) + assert.Equal(t, tc.want.Allowed, got.Allowed) + assert.Equal(t, tc.want.Error, got.Error) + assert.Equal(t, tc.want.Causes, got.Causes) + }) + } +} + +func TestGetVMImages(t *testing.T) { + testCases := []struct { + name string + client *mockV4Client + id *capxv1.NutanixResourceIdentifier + want []vmmv4.Image + wantErr bool + errorMsg string + }{ + { + name: "get image by uuid success", + client: &mockV4Client{ + getImageByIdFunc: func(uuid *string) (*vmmv4.GetImageApiResponse, error) { + assert.Equal(t, "test-uuid", *uuid) + resp := &vmmv4.GetImageApiResponse{} + err := resp.SetData(vmmv4.Image{ + ObjectType_: ptr.To("vmm.v4.content.Image"), + ExtId: ptr.To("test-uuid"), + }) + require.NoError(t, err) + return resp, nil + }, + }, + id: &capxv1.NutanixResourceIdentifier{ + Type: capxv1.NutanixIdentifierUUID, + UUID: ptr.To("test-uuid"), + }, + want: []vmmv4.Image{ + { + ObjectType_: ptr.To("vmm.v4.content.Image"), + ExtId: ptr.To("test-uuid"), + }, + }, + wantErr: false, + }, + { + name: "get image by name success", + client: &mockV4Client{ + listImagesFunc: func(page, + limit *int, + filter, + orderby, + select_ *string, + args ...map[string]interface{}, + ) ( + *vmmv4.ListImagesApiResponse, + error, + ) { + assert.NotNil(t, filter) + assert.Equal(t, "name eq 'test-name'", *filter) + resp := &vmmv4.ListImagesApiResponse{} + err := resp.SetData([]vmmv4.Image{ + { + Name: ptr.To("test-name"), + }, + }) + require.NoError(t, err) + return resp, nil + }, + }, + id: &capxv1.NutanixResourceIdentifier{ + Type: capxv1.NutanixIdentifierName, + Name: ptr.To("test-name"), + }, + want: []vmmv4.Image{ + { + Name: ptr.To("test-name"), + }, + }, + wantErr: false, + }, + { + name: "get image by uuid error", + client: &mockV4Client{ + getImageByIdFunc: func(uuid *string) (*vmmv4.GetImageApiResponse, error) { + return nil, fmt.Errorf("api error") + }, + }, + id: &capxv1.NutanixResourceIdentifier{ + Type: capxv1.NutanixIdentifierUUID, + UUID: ptr.To("test-uuid"), + }, + wantErr: true, + errorMsg: "api error", + }, + { + name: "get image by name error", + client: &mockV4Client{ + listImagesFunc: func(page, + limit *int, + filter, + orderby, + select_ *string, + args ...map[string]interface{}, + ) ( + *vmmv4.ListImagesApiResponse, + error, + ) { + return nil, fmt.Errorf("api error") + }, + }, + id: &capxv1.NutanixResourceIdentifier{ + Type: capxv1.NutanixIdentifierName, + Name: ptr.To("test-name"), + }, + wantErr: true, + errorMsg: "api error", + }, + { + name: "neither name nor uuid specified", + client: &mockV4Client{}, + id: &capxv1.NutanixResourceIdentifier{ + // Both Name and UUID are not set + }, + wantErr: true, + errorMsg: "image identifier is missing both name and uuid", + }, + { + name: "invalid data from GetImageById", + client: &mockV4Client{ + getImageByIdFunc: func(uuid *string) (*vmmv4.GetImageApiResponse, error) { + return &vmmv4.GetImageApiResponse{ + Data: &vmmv4.OneOfGetImageApiResponseData{ + ObjectType_: ptr.To("wrong-type"), + }, + }, nil + }, + }, + id: &capxv1.NutanixResourceIdentifier{ + Type: capxv1.NutanixIdentifierUUID, + UUID: ptr.To("test-uuid"), + }, + wantErr: true, + errorMsg: "failed to get data returned by GetImageById", + }, + { + name: "empty response from ListImages", + client: &mockV4Client{ + listImagesFunc: func(page, + limit *int, + filter, + orderby, + select_ *string, + args ...map[string]interface{}, + ) ( + *vmmv4.ListImagesApiResponse, + error, + ) { + return &vmmv4.ListImagesApiResponse{ + Data: nil, // Empty data + }, nil + }, + }, + id: &capxv1.NutanixResourceIdentifier{ + Type: capxv1.NutanixIdentifierName, + Name: ptr.To("test-name"), + }, + want: []vmmv4.Image{}, + wantErr: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got, err := getVMImages(tc.client, tc.id) + + if tc.wantErr { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.errorMsg) + } else { + require.NoError(t, err) + assert.Equal(t, tc.want, got) + } + }) + } +} + +func TestInitVMImageChecks(t *testing.T) { + testCases := []struct { + name string + nutanixClusterConfigSpec *carenv1.NutanixClusterConfigSpec + nutanixWorkerNodeConfigSpecByMDName map[string]*carenv1.NutanixWorkerNodeConfigSpec + expectedChecks int + expectedControlPlaneCheckFieldIncluded bool + expectedWorkerNodeCheckFieldPatternExists bool + }{ + { + name: "no nutanix configuration", + nutanixClusterConfigSpec: nil, + nutanixWorkerNodeConfigSpecByMDName: nil, + expectedChecks: 0, + expectedControlPlaneCheckFieldIncluded: false, + expectedWorkerNodeCheckFieldPatternExists: false, + }, + { + name: "control plane configuration only", + nutanixClusterConfigSpec: &carenv1.NutanixClusterConfigSpec{ + ControlPlane: &carenv1.NutanixControlPlaneSpec{ + Nutanix: &carenv1.NutanixNodeSpec{ + MachineDetails: carenv1.NutanixMachineDetails{ + Image: &capxv1.NutanixResourceIdentifier{ + Type: capxv1.NutanixIdentifierUUID, + UUID: ptr.To("test-uuid"), + }, + }, + }, + }, + }, + nutanixWorkerNodeConfigSpecByMDName: nil, + expectedChecks: 1, + expectedControlPlaneCheckFieldIncluded: true, + expectedWorkerNodeCheckFieldPatternExists: false, + }, + { + name: "worker nodes configuration only", + nutanixClusterConfigSpec: nil, + nutanixWorkerNodeConfigSpecByMDName: map[string]*carenv1.NutanixWorkerNodeConfigSpec{ + "worker-1": { + Nutanix: &carenv1.NutanixNodeSpec{ + MachineDetails: carenv1.NutanixMachineDetails{ + Image: &capxv1.NutanixResourceIdentifier{ + Type: capxv1.NutanixIdentifierName, + Name: ptr.To("worker-image"), + }, + }, + }, + }, + }, + expectedChecks: 1, + expectedControlPlaneCheckFieldIncluded: false, + expectedWorkerNodeCheckFieldPatternExists: true, + }, + { + name: "both control plane and worker nodes configuration", + nutanixClusterConfigSpec: &carenv1.NutanixClusterConfigSpec{ + ControlPlane: &carenv1.NutanixControlPlaneSpec{ + Nutanix: &carenv1.NutanixNodeSpec{ + MachineDetails: carenv1.NutanixMachineDetails{ + Image: &capxv1.NutanixResourceIdentifier{ + Type: capxv1.NutanixIdentifierUUID, + UUID: ptr.To("cp-uuid"), + }, + }, + }, + }, + }, + nutanixWorkerNodeConfigSpecByMDName: map[string]*carenv1.NutanixWorkerNodeConfigSpec{ + "worker-1": { + Nutanix: &carenv1.NutanixNodeSpec{ + MachineDetails: carenv1.NutanixMachineDetails{ + Image: &capxv1.NutanixResourceIdentifier{ + Type: capxv1.NutanixIdentifierName, + Name: ptr.To("worker1-image"), + }, + }, + }, + }, + "worker-2": { + Nutanix: &carenv1.NutanixNodeSpec{ + MachineDetails: carenv1.NutanixMachineDetails{ + Image: &capxv1.NutanixResourceIdentifier{ + Type: capxv1.NutanixIdentifierName, + Name: ptr.To("worker2-image"), + }, + }, + }, + }, + }, + expectedChecks: 3, // 1 control plane + 2 workers + expectedControlPlaneCheckFieldIncluded: true, + expectedWorkerNodeCheckFieldPatternExists: true, + }, + { + name: "worker with nil Nutanix config", + nutanixClusterConfigSpec: nil, + nutanixWorkerNodeConfigSpecByMDName: map[string]*carenv1.NutanixWorkerNodeConfigSpec{ + "worker-1": { + Nutanix: nil, + }, + "worker-2": { + Nutanix: &carenv1.NutanixNodeSpec{ + MachineDetails: carenv1.NutanixMachineDetails{ + Image: &capxv1.NutanixResourceIdentifier{ + Type: capxv1.NutanixIdentifierName, + Name: ptr.To("worker2-image"), + }, + }, + }, + }, + }, + expectedChecks: 1, // only worker-2 + expectedControlPlaneCheckFieldIncluded: false, + expectedWorkerNodeCheckFieldPatternExists: true, + }, + { + name: "control plane with nil Nutanix config", + nutanixClusterConfigSpec: &carenv1.NutanixClusterConfigSpec{ + ControlPlane: &carenv1.NutanixControlPlaneSpec{ + Nutanix: nil, // null nutanix config + }, + }, + nutanixWorkerNodeConfigSpecByMDName: nil, + expectedChecks: 0, + expectedControlPlaneCheckFieldIncluded: false, + expectedWorkerNodeCheckFieldPatternExists: false, + }, + { + name: "null control plane config", + nutanixClusterConfigSpec: &carenv1.NutanixClusterConfigSpec{ + ControlPlane: nil, // null control plane + }, + nutanixWorkerNodeConfigSpecByMDName: nil, + expectedChecks: 0, + expectedControlPlaneCheckFieldIncluded: false, + expectedWorkerNodeCheckFieldPatternExists: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + logger := testr.New(t) + checker := &nutanixChecker{ + log: logger, + nutanixClusterConfigSpec: tc.nutanixClusterConfigSpec, + nutanixWorkerNodeConfigSpecByMachineDeploymentName: tc.nutanixWorkerNodeConfigSpecByMDName, + } + + // Trap the vmImageCheck calls to verify field paths + var capturedFields []string + checker.vmImageCheckFunc = func( + n *nutanixChecker, + machineDetails *carenv1.NutanixMachineDetails, + field string, + ) preflight.Check { + capturedFields = append(capturedFields, field) + return func(ctx context.Context) preflight.CheckResult { + // Simulate a successful check + return preflight.CheckResult{ + Name: "NutanixVMImage", + Allowed: true, + } + } + } + + // Call the method under test + checker.initVMImageChecksFunc = initVMImageChecks + checks := checker.initVMImageChecksFunc(checker) + + // Verify number of checks + assert.Len(t, checks, tc.expectedChecks) + assert.Len(t, capturedFields, tc.expectedChecks) + + // Verify field names in checks + if tc.expectedControlPlaneCheckFieldIncluded { + assert.Contains( + t, + capturedFields, + "cluster.spec.topology[.name=clusterConfig].value.controlPlane.nutanix.machineDetails", + ) + } + + if tc.expectedWorkerNodeCheckFieldPatternExists { + foundWorkerFieldPattern := false + for _, field := range capturedFields { + if field != "cluster.spec.topology[.name=clusterConfig].value.controlPlane.nutanix.machineDetails" { + // This is not the control plane field, so it must be a worker node field + assert.Contains(t, field, "cluster.spec.topology.workers.machineDeployments[.name=") + assert.Contains(t, field, "].variables[.name=workerConfig].value.nutanix.machineDetails") + foundWorkerFieldPattern = true + } + } + assert.True(t, foundWorkerFieldPattern, "Worker node field pattern not found in any checks") + } + }) + } +} diff --git a/pkg/webhook/preflight/nutanix/specs.go b/pkg/webhook/preflight/nutanix/specs.go index 78796223f..9cd2a2e0d 100644 --- a/pkg/webhook/preflight/nutanix/specs.go +++ b/pkg/webhook/preflight/nutanix/specs.go @@ -12,7 +12,9 @@ import ( "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight" ) -func (n *nutanixChecker) initNutanixConfiguration() preflight.Check { +func initNutanixConfiguration( + n *nutanixChecker, +) preflight.Check { n.log.V(5).Info("Initializing Nutanix configuration check") result := preflight.CheckResult{ @@ -70,7 +72,11 @@ func (n *nutanixChecker) initNutanixConfiguration() preflight.Check { carenv1.WorkerConfigVariableName, err, ), - Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix", + Field: fmt.Sprintf( + "cluster.spec.topology.workers.machineDeployments[.name=%s]"+ + ".variables[.name=workerConfig].value.nutanix.machineDetails", + md.Name, + ), }, ) } diff --git a/pkg/webhook/preflight/nutanix/specs_test.go b/pkg/webhook/preflight/nutanix/specs_test.go new file mode 100644 index 000000000..2cb950760 --- /dev/null +++ b/pkg/webhook/preflight/nutanix/specs_test.go @@ -0,0 +1,387 @@ +// Copyright 2025 Nutanix. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package nutanix + +import ( + "context" + "testing" + + "github.com/go-logr/logr/testr" + "github.com/stretchr/testify/assert" + v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + + carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" + "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight" +) + +func TestNutanixChecker_initNutanixConfiguration(t *testing.T) { + tests := []struct { + name string + cluster *clusterv1.Cluster + expectedResult preflight.CheckResult + expectedNutanixClusterConfigSpec bool + expectedWorkerNodeConfigSpecMapNotEmpty bool + expectedWorkerNodeConfigSpecMapEntryCount int + }{ + { + name: "valid cluster config", + cluster: &clusterv1.Cluster{ + Spec: clusterv1.ClusterSpec{ + Topology: &clusterv1.Topology{ + Variables: []clusterv1.ClusterVariable{ + { + Name: carenv1.ClusterConfigVariableName, + Value: v1.JSON{ + Raw: []byte(`{"nutanix": {"prismCentral": {"address": "pc.example.com"}}}`), + }, + }, + }, + }, + }, + }, + expectedResult: preflight.CheckResult{ + Name: "NutanixCredentials", + Allowed: true, + }, + expectedNutanixClusterConfigSpec: true, + expectedWorkerNodeConfigSpecMapNotEmpty: false, + expectedWorkerNodeConfigSpecMapEntryCount: 0, + }, + { + name: "valid control plane config", + cluster: &clusterv1.Cluster{ + Spec: clusterv1.ClusterSpec{ + Topology: &clusterv1.Topology{ + Variables: []clusterv1.ClusterVariable{ + { + Name: carenv1.ClusterConfigVariableName, + Value: v1.JSON{ + Raw: []byte( + `{"controlPlane": {"nutanix": {"prismElement": {"address": "pe.example.com"}}}}`, + ), + }, + }, + }, + }, + }, + }, + expectedResult: preflight.CheckResult{ + Name: "NutanixCredentials", + Allowed: true, + }, + expectedNutanixClusterConfigSpec: true, + expectedWorkerNodeConfigSpecMapNotEmpty: false, + expectedWorkerNodeConfigSpecMapEntryCount: 0, + }, + { + name: "valid worker config", + cluster: &clusterv1.Cluster{ + Spec: clusterv1.ClusterSpec{ + Topology: &clusterv1.Topology{ + Variables: []clusterv1.ClusterVariable{ + { + Name: carenv1.ClusterConfigVariableName, + Value: v1.JSON{ + Raw: []byte(`{}`), + }, + }, + }, + Workers: &clusterv1.WorkersTopology{ + MachineDeployments: []clusterv1.MachineDeploymentTopology{ + { + Name: "md-0", + Variables: &clusterv1.MachineDeploymentVariables{ + Overrides: []clusterv1.ClusterVariable{ + { + Name: carenv1.WorkerConfigVariableName, + Value: v1.JSON{ + Raw: []byte( + `{"nutanix": {"prismElement": {"address": "pe.example.com"}}}`, + ), + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + expectedResult: preflight.CheckResult{ + Name: "NutanixCredentials", + Allowed: true, + }, + expectedNutanixClusterConfigSpec: false, + expectedWorkerNodeConfigSpecMapNotEmpty: true, + expectedWorkerNodeConfigSpecMapEntryCount: 1, + }, + { + name: "invalid cluster config", + cluster: &clusterv1.Cluster{ + Spec: clusterv1.ClusterSpec{ + Topology: &clusterv1.Topology{ + Variables: []clusterv1.ClusterVariable{ + { + Name: carenv1.ClusterConfigVariableName, + Value: v1.JSON{ + Raw: []byte(`{invalid-json`), + }, + }, + }, + }, + }, + }, + expectedResult: preflight.CheckResult{ + Name: "NutanixCredentials", + Allowed: false, + Error: true, + Causes: []preflight.Cause{ + { + Message: "Failed to unmarshal cluster variable clusterConfig: failed to unmarshal json:" + + " invalid character 'i' looking for beginning of object key string", + Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix", + }, + }, + }, + expectedNutanixClusterConfigSpec: false, + expectedWorkerNodeConfigSpecMapNotEmpty: false, + expectedWorkerNodeConfigSpecMapEntryCount: 0, + }, + { + name: "invalid worker config", + cluster: &clusterv1.Cluster{ + Spec: clusterv1.ClusterSpec{ + Topology: &clusterv1.Topology{ + Variables: []clusterv1.ClusterVariable{ + { + Name: carenv1.ClusterConfigVariableName, + Value: v1.JSON{ + Raw: []byte(`{}`), + }, + }, + }, + Workers: &clusterv1.WorkersTopology{ + MachineDeployments: []clusterv1.MachineDeploymentTopology{ + { + Name: "md-0", + Variables: &clusterv1.MachineDeploymentVariables{ + Overrides: []clusterv1.ClusterVariable{ + { + Name: carenv1.WorkerConfigVariableName, + Value: v1.JSON{ + Raw: []byte(`{invalid-json`), + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + expectedResult: preflight.CheckResult{ + Name: "NutanixCredentials", + Allowed: false, + Error: true, + Causes: []preflight.Cause{ + { + Message: "Failed to unmarshal topology machineDeployment variable workerConfig:" + + " failed to unmarshal json: invalid character 'i' looking for beginning of object key string", + Field: "cluster.spec.topology.workers.machineDeployments[.name=md-0]." + + "variables[.name=workerConfig].value.nutanix.machineDetails", + }, + }, + }, + expectedNutanixClusterConfigSpec: false, + expectedWorkerNodeConfigSpecMapNotEmpty: false, + expectedWorkerNodeConfigSpecMapEntryCount: 0, + }, + { + name: "no nutanix config", + cluster: &clusterv1.Cluster{ + Spec: clusterv1.ClusterSpec{ + Topology: &clusterv1.Topology{ + Variables: []clusterv1.ClusterVariable{ + { + Name: carenv1.ClusterConfigVariableName, + Value: v1.JSON{ + Raw: []byte(`{}`), + }, + }, + }, + }, + }, + }, + expectedResult: preflight.CheckResult{ + Name: "NutanixCredentials", + Allowed: true, + }, + expectedNutanixClusterConfigSpec: false, + expectedWorkerNodeConfigSpecMapNotEmpty: false, + expectedWorkerNodeConfigSpecMapEntryCount: 0, + }, + { + name: "multiple worker configs", + cluster: &clusterv1.Cluster{ + Spec: clusterv1.ClusterSpec{ + Topology: &clusterv1.Topology{ + Variables: []clusterv1.ClusterVariable{ + { + Name: carenv1.ClusterConfigVariableName, + Value: v1.JSON{ + Raw: []byte(`{}`), + }, + }, + }, + Workers: &clusterv1.WorkersTopology{ + MachineDeployments: []clusterv1.MachineDeploymentTopology{ + { + Name: "md-0", + Variables: &clusterv1.MachineDeploymentVariables{ + Overrides: []clusterv1.ClusterVariable{ + { + Name: carenv1.WorkerConfigVariableName, + Value: v1.JSON{ + Raw: []byte( + `{"nutanix": {"prismElement": {"address": "pe1.example.com"}}}`, + ), + }, + }, + }, + }, + }, + { + Name: "md-1", + Variables: &clusterv1.MachineDeploymentVariables{ + Overrides: []clusterv1.ClusterVariable{ + { + Name: carenv1.WorkerConfigVariableName, + Value: v1.JSON{ + Raw: []byte( + `{"nutanix": {"prismElement": {"address": "pe2.example.com"}}}`, + ), + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + expectedResult: preflight.CheckResult{ + Name: "NutanixCredentials", + Allowed: true, + }, + expectedNutanixClusterConfigSpec: false, + expectedWorkerNodeConfigSpecMapNotEmpty: true, + expectedWorkerNodeConfigSpecMapEntryCount: 2, + }, + { + name: "worker config without nutanix field", + cluster: &clusterv1.Cluster{ + Spec: clusterv1.ClusterSpec{ + Topology: &clusterv1.Topology{ + Variables: []clusterv1.ClusterVariable{ + { + Name: carenv1.ClusterConfigVariableName, + Value: v1.JSON{ + Raw: []byte(`{}`), + }, + }, + }, + Workers: &clusterv1.WorkersTopology{ + MachineDeployments: []clusterv1.MachineDeploymentTopology{ + { + Name: "md-0", + Variables: &clusterv1.MachineDeploymentVariables{ + Overrides: []clusterv1.ClusterVariable{ + { + Name: carenv1.WorkerConfigVariableName, + Value: v1.JSON{ + Raw: []byte(`{"someOtherField": true}`), + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + expectedResult: preflight.CheckResult{ + Name: "NutanixCredentials", + Allowed: true, + }, + expectedNutanixClusterConfigSpec: false, + expectedWorkerNodeConfigSpecMapNotEmpty: false, + expectedWorkerNodeConfigSpecMapEntryCount: 0, + }, + { + name: "machineDeployment without variables", + cluster: &clusterv1.Cluster{ + Spec: clusterv1.ClusterSpec{ + Topology: &clusterv1.Topology{ + Variables: []clusterv1.ClusterVariable{ + { + Name: carenv1.ClusterConfigVariableName, + Value: v1.JSON{ + Raw: []byte(`{}`), + }, + }, + }, + Workers: &clusterv1.WorkersTopology{ + MachineDeployments: []clusterv1.MachineDeploymentTopology{ + { + Name: "md-0", + }, + }, + }, + }, + }, + }, + expectedResult: preflight.CheckResult{ + Name: "NutanixCredentials", + Allowed: true, + }, + expectedNutanixClusterConfigSpec: false, + expectedWorkerNodeConfigSpecMapNotEmpty: false, + expectedWorkerNodeConfigSpecMapEntryCount: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + logger := testr.New(t) + + n := &nutanixChecker{ + log: logger, + cluster: tt.cluster, + } + + checkFunc := initNutanixConfiguration(n) + result := checkFunc(context.Background()) + + assert.Equal(t, tt.expectedResult, result) + + hasNutanixClusterConfigSpec := n.nutanixClusterConfigSpec != nil + assert.Equal(t, tt.expectedNutanixClusterConfigSpec, hasNutanixClusterConfigSpec) + + hasWorkerNodeConfigSpecMap := n.nutanixWorkerNodeConfigSpecByMachineDeploymentName != nil + assert.Equal(t, tt.expectedWorkerNodeConfigSpecMapNotEmpty, hasWorkerNodeConfigSpecMap) + + if hasWorkerNodeConfigSpecMap { + assert.Len( + t, + n.nutanixWorkerNodeConfigSpecByMachineDeploymentName, tt.expectedWorkerNodeConfigSpecMapEntryCount, + ) + } + }) + } +} From d8bf98082689fd37487c0aaf74a75b89d2af0cb4 Mon Sep 17 00:00:00 2001 From: Daniel Lipovetsky Date: Mon, 9 Jun 2025 17:24:34 -0700 Subject: [PATCH 05/12] fixup! fixup! feat: Nutanix VM image preflight check Use the correct check name --- pkg/webhook/preflight/nutanix/specs.go | 2 +- pkg/webhook/preflight/nutanix/specs_test.go | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/pkg/webhook/preflight/nutanix/specs.go b/pkg/webhook/preflight/nutanix/specs.go index 9cd2a2e0d..12b211077 100644 --- a/pkg/webhook/preflight/nutanix/specs.go +++ b/pkg/webhook/preflight/nutanix/specs.go @@ -18,7 +18,7 @@ func initNutanixConfiguration( n.log.V(5).Info("Initializing Nutanix configuration check") result := preflight.CheckResult{ - Name: "NutanixCredentials", + Name: "NutanixConfiguration", Allowed: true, } diff --git a/pkg/webhook/preflight/nutanix/specs_test.go b/pkg/webhook/preflight/nutanix/specs_test.go index 2cb950760..ca669e991 100644 --- a/pkg/webhook/preflight/nutanix/specs_test.go +++ b/pkg/webhook/preflight/nutanix/specs_test.go @@ -42,7 +42,7 @@ func TestNutanixChecker_initNutanixConfiguration(t *testing.T) { }, }, expectedResult: preflight.CheckResult{ - Name: "NutanixCredentials", + Name: "NutanixConfiguration", Allowed: true, }, expectedNutanixClusterConfigSpec: true, @@ -68,7 +68,7 @@ func TestNutanixChecker_initNutanixConfiguration(t *testing.T) { }, }, expectedResult: preflight.CheckResult{ - Name: "NutanixCredentials", + Name: "NutanixConfiguration", Allowed: true, }, expectedNutanixClusterConfigSpec: true, @@ -111,7 +111,7 @@ func TestNutanixChecker_initNutanixConfiguration(t *testing.T) { }, }, expectedResult: preflight.CheckResult{ - Name: "NutanixCredentials", + Name: "NutanixConfiguration", Allowed: true, }, expectedNutanixClusterConfigSpec: false, @@ -135,7 +135,7 @@ func TestNutanixChecker_initNutanixConfiguration(t *testing.T) { }, }, expectedResult: preflight.CheckResult{ - Name: "NutanixCredentials", + Name: "NutanixConfiguration", Allowed: false, Error: true, Causes: []preflight.Cause{ @@ -184,7 +184,7 @@ func TestNutanixChecker_initNutanixConfiguration(t *testing.T) { }, }, expectedResult: preflight.CheckResult{ - Name: "NutanixCredentials", + Name: "NutanixConfiguration", Allowed: false, Error: true, Causes: []preflight.Cause{ @@ -217,7 +217,7 @@ func TestNutanixChecker_initNutanixConfiguration(t *testing.T) { }, }, expectedResult: preflight.CheckResult{ - Name: "NutanixCredentials", + Name: "NutanixConfiguration", Allowed: true, }, expectedNutanixClusterConfigSpec: false, @@ -275,7 +275,7 @@ func TestNutanixChecker_initNutanixConfiguration(t *testing.T) { }, }, expectedResult: preflight.CheckResult{ - Name: "NutanixCredentials", + Name: "NutanixConfiguration", Allowed: true, }, expectedNutanixClusterConfigSpec: false, @@ -316,7 +316,7 @@ func TestNutanixChecker_initNutanixConfiguration(t *testing.T) { }, }, expectedResult: preflight.CheckResult{ - Name: "NutanixCredentials", + Name: "NutanixConfiguration", Allowed: true, }, expectedNutanixClusterConfigSpec: false, @@ -347,7 +347,7 @@ func TestNutanixChecker_initNutanixConfiguration(t *testing.T) { }, }, expectedResult: preflight.CheckResult{ - Name: "NutanixCredentials", + Name: "NutanixConfiguration", Allowed: true, }, expectedNutanixClusterConfigSpec: false, From ad484cf3879161d6c709b920e5fe72e926ff12de Mon Sep 17 00:00:00 2001 From: Daniel Lipovetsky Date: Wed, 11 Jun 2025 14:52:59 -0700 Subject: [PATCH 06/12] fixup! feat: Nutanix VM image preflight check Run VM Image check only if Nutanix clients are initialized. This avoids returning errors that are not actionable. The user will see an error from the credentials check only. --- pkg/webhook/preflight/nutanix/checker.go | 2 - pkg/webhook/preflight/nutanix/credentials.go | 44 +++++++++----- .../preflight/nutanix/credentials_test.go | 2 +- pkg/webhook/preflight/nutanix/image.go | 17 ++---- pkg/webhook/preflight/nutanix/image_test.go | 60 +++++++++---------- 5 files changed, 62 insertions(+), 63 deletions(-) diff --git a/pkg/webhook/preflight/nutanix/checker.go b/pkg/webhook/preflight/nutanix/checker.go index 8fb047191..4d8fdddc1 100644 --- a/pkg/webhook/preflight/nutanix/checker.go +++ b/pkg/webhook/preflight/nutanix/checker.go @@ -39,8 +39,6 @@ type nutanixChecker struct { nutanixClusterConfigSpec *carenv1.NutanixClusterConfigSpec nutanixWorkerNodeConfigSpecByMachineDeploymentName map[string]*carenv1.NutanixWorkerNodeConfigSpec - credentials prismgoclient.Credentials - v3client v3client v3clientFactory func(prismgoclient.Credentials) (v3client, error) diff --git a/pkg/webhook/preflight/nutanix/credentials.go b/pkg/webhook/preflight/nutanix/credentials.go index 31b616799..7122b1ef6 100644 --- a/pkg/webhook/preflight/nutanix/credentials.go +++ b/pkg/webhook/preflight/nutanix/credentials.go @@ -146,7 +146,7 @@ func initCredentialsCheck( } // Initialize the credentials. - n.credentials = prismgoclient.Credentials{ + credentials := prismgoclient.Credentials{ Endpoint: fmt.Sprintf("%s:%d", host, port), URL: fmt.Sprintf("https://%s:%d", host, port), Username: usernamePassword.Username, @@ -155,7 +155,7 @@ func initCredentialsCheck( } // Initialize the clients. - n.v4client, err = n.v4clientFactory(n.credentials) + v4client, err := n.v4clientFactory(credentials) if err != nil { result.Allowed = false result.Error = true @@ -167,7 +167,7 @@ func initCredentialsCheck( ) } - n.v3client, err = n.v3clientFactory(n.credentials) + v3client, err := n.v3clientFactory(credentials) if err != nil { result.Allowed = false result.Error = true @@ -177,21 +177,35 @@ func initCredentialsCheck( Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials", }, ) - } else { - // Validate the credentials using an API call. - _, err = n.v3client.GetCurrentLoggedInUser(ctx) - if err != nil { - result.Allowed = false - result.Error = true - result.Causes = append(result.Causes, - preflight.Cause{ - Message: fmt.Sprintf("failed to validate credentials using the v3 API client: %s", err), - Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials", - }, - ) + } + + if v3client == nil || v4client == nil { + return func(ctx context.Context) preflight.CheckResult { + return result } } + // Validate the credentials using an API call. + _, err = v3client.GetCurrentLoggedInUser(ctx) + if err != nil { + result.Allowed = false + result.Error = true + result.Causes = append(result.Causes, + preflight.Cause{ + Message: fmt.Sprintf("Failed to validate credentials using the v3 API client. "+ + "The URL and/or credentials may be incorrect. (Error: %q)", err), + Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint", + }, + ) + return func(ctx context.Context) preflight.CheckResult { + return result + } + } + + // We initialized both clients, and verified the credentials using the v3 client. + n.v3client = v3client + n.v4client = v4client + return func(ctx context.Context) preflight.CheckResult { return result } diff --git a/pkg/webhook/preflight/nutanix/credentials_test.go b/pkg/webhook/preflight/nutanix/credentials_test.go index 40ad68426..8c458867b 100644 --- a/pkg/webhook/preflight/nutanix/credentials_test.go +++ b/pkg/webhook/preflight/nutanix/credentials_test.go @@ -174,7 +174,7 @@ func TestInitCredentialsCheck_FailedToGetCurrentLoggedInUser(t *testing.T) { result := check(context.Background()) assert.False(t, result.Allowed) assert.True(t, result.Error) - assert.Contains(t, result.Causes[0].Message, "failed to validate credentials using the v3 API client") + assert.Contains(t, result.Causes[0].Message, "Failed to validate credentials using the v3 API client.") } func validNutanixChecker() *nutanixChecker { diff --git a/pkg/webhook/preflight/nutanix/image.go b/pkg/webhook/preflight/nutanix/image.go index b2c9542c6..9eadca2f1 100644 --- a/pkg/webhook/preflight/nutanix/image.go +++ b/pkg/webhook/preflight/nutanix/image.go @@ -19,6 +19,10 @@ func initVMImageChecks( ) []preflight.Check { checks := []preflight.Check{} + if n.v4client == nil { + return checks + } + if n.nutanixClusterConfigSpec != nil && n.nutanixClusterConfigSpec.ControlPlane != nil && n.nutanixClusterConfigSpec.ControlPlane.Nutanix != nil { checks = append(checks, @@ -62,19 +66,6 @@ func vmImageCheck( Allowed: false, } - // If the v4 client is not initialized, we cannot perform VM image checks. - if n.v4client == nil { - result.Allowed = false - result.Error = true - result.Causes = append(result.Causes, - preflight.Cause{ - Message: "Nutanix v4 client is not initialized, cannot perform VM image checks", - Field: "", - }, - ) - return result - } - if machineDetails.ImageLookup != nil { result.Allowed = true result.Warnings = append( diff --git a/pkg/webhook/preflight/nutanix/image_test.go b/pkg/webhook/preflight/nutanix/image_test.go index ce771f9a3..355557150 100644 --- a/pkg/webhook/preflight/nutanix/image_test.go +++ b/pkg/webhook/preflight/nutanix/image_test.go @@ -59,27 +59,6 @@ func TestVMImageCheck(t *testing.T) { machineDetails *carenv1.NutanixMachineDetails want preflight.CheckResult }{ - { - name: "Nutanix v4 client not initialized", - v4client: nil, - machineDetails: &carenv1.NutanixMachineDetails{ - Image: &capxv1.NutanixResourceIdentifier{ - Type: capxv1.NutanixIdentifierUUID, - UUID: ptr.To("test-uuid"), - }, - }, - want: preflight.CheckResult{ - Name: "NutanixVMImage", - Allowed: false, - Error: true, - Causes: []preflight.Cause{ - { - Message: "Nutanix v4 client is not initialized, cannot perform VM image checks", - Field: "", - }, - }, - }, - }, { name: "imageLookup not yet supported", v4client: &mockV4Client{}, @@ -514,16 +493,27 @@ func TestInitVMImageChecks(t *testing.T) { name string nutanixClusterConfigSpec *carenv1.NutanixClusterConfigSpec nutanixWorkerNodeConfigSpecByMDName map[string]*carenv1.NutanixWorkerNodeConfigSpec + v4client v4client expectedChecks int expectedControlPlaneCheckFieldIncluded bool expectedWorkerNodeCheckFieldPatternExists bool }{ { - name: "no nutanix configuration", - nutanixClusterConfigSpec: nil, - nutanixWorkerNodeConfigSpecByMDName: nil, - expectedChecks: 0, - expectedControlPlaneCheckFieldIncluded: false, + name: "client not initialized", + nutanixClusterConfigSpec: nil, + nutanixWorkerNodeConfigSpecByMDName: nil, + v4client: nil, + expectedChecks: 0, + expectedControlPlaneCheckFieldIncluded: false, + expectedWorkerNodeCheckFieldPatternExists: false, + }, + { + name: "no nutanix configuration", + nutanixClusterConfigSpec: nil, + nutanixWorkerNodeConfigSpecByMDName: nil, + v4client: &mockv4client{}, + expectedChecks: 0, + expectedControlPlaneCheckFieldIncluded: false, expectedWorkerNodeCheckFieldPatternExists: false, }, { @@ -541,6 +531,7 @@ func TestInitVMImageChecks(t *testing.T) { }, }, nutanixWorkerNodeConfigSpecByMDName: nil, + v4client: &mockv4client{}, expectedChecks: 1, expectedControlPlaneCheckFieldIncluded: true, expectedWorkerNodeCheckFieldPatternExists: false, @@ -560,8 +551,9 @@ func TestInitVMImageChecks(t *testing.T) { }, }, }, - expectedChecks: 1, - expectedControlPlaneCheckFieldIncluded: false, + v4client: &mockv4client{}, + expectedChecks: 1, + expectedControlPlaneCheckFieldIncluded: false, expectedWorkerNodeCheckFieldPatternExists: true, }, { @@ -600,8 +592,9 @@ func TestInitVMImageChecks(t *testing.T) { }, }, }, - expectedChecks: 3, // 1 control plane + 2 workers - expectedControlPlaneCheckFieldIncluded: true, + v4client: &mockv4client{}, + expectedChecks: 3, // 1 control plane + 2 workers + expectedControlPlaneCheckFieldIncluded: true, expectedWorkerNodeCheckFieldPatternExists: true, }, { @@ -622,8 +615,9 @@ func TestInitVMImageChecks(t *testing.T) { }, }, }, - expectedChecks: 1, // only worker-2 - expectedControlPlaneCheckFieldIncluded: false, + v4client: &mockv4client{}, + expectedChecks: 1, // only worker-2 + expectedControlPlaneCheckFieldIncluded: false, expectedWorkerNodeCheckFieldPatternExists: true, }, { @@ -644,6 +638,7 @@ func TestInitVMImageChecks(t *testing.T) { ControlPlane: nil, // null control plane }, nutanixWorkerNodeConfigSpecByMDName: nil, + v4client: &mockv4client{}, expectedChecks: 0, expectedControlPlaneCheckFieldIncluded: false, expectedWorkerNodeCheckFieldPatternExists: false, @@ -657,6 +652,7 @@ func TestInitVMImageChecks(t *testing.T) { log: logger, nutanixClusterConfigSpec: tc.nutanixClusterConfigSpec, nutanixWorkerNodeConfigSpecByMachineDeploymentName: tc.nutanixWorkerNodeConfigSpecByMDName, + v4client: tc.v4client, } // Trap the vmImageCheck calls to verify field paths From 4273c03d6d9849e79c239a625d2f651f517c82d5 Mon Sep 17 00:00:00 2001 From: Daniel Lipovetsky Date: Wed, 11 Jun 2025 15:43:57 -0700 Subject: [PATCH 07/12] fixup! feat: Nutanix VM image preflight check Remove unused v4 client mock --- .../preflight/nutanix/credentials_test.go | 32 ++++++--- pkg/webhook/preflight/nutanix/image_test.go | 65 +++++-------------- 2 files changed, 40 insertions(+), 57 deletions(-) diff --git a/pkg/webhook/preflight/nutanix/credentials_test.go b/pkg/webhook/preflight/nutanix/credentials_test.go index 8c458867b..9c0182a84 100644 --- a/pkg/webhook/preflight/nutanix/credentials_test.go +++ b/pkg/webhook/preflight/nutanix/credentials_test.go @@ -243,19 +243,35 @@ func (m *mockv3client) GetCurrentLoggedInUser(ctx context.Context) (*prismv3.Use return m.user, m.err } +// mockv4client is a mock implementation of the v4client interface for testing. type mockv4client struct { - image *vmmv4.GetImageApiResponse - images *vmmv4.ListImagesApiResponse + getImageByIdFunc func( + uuid *string, + ) ( + *vmmv4.GetImageApiResponse, error, + ) + + listImagesFunc func( + page, + limit *int, + filter, + orderby, + select_ *string, + args ...map[string]interface{}, + ) ( + *vmmv4.ListImagesApiResponse, + error, + ) } -func (m *mockv4client) GetImageById(id *string) (*vmmv4.GetImageApiResponse, error) { - return m.image, nil +func (m *mockv4client) GetImageById(uuid *string) (*vmmv4.GetImageApiResponse, error) { + return m.getImageByIdFunc(uuid) } func (m *mockv4client) ListImages( - _, _ *int, - _, _, _ *string, - _ ...map[string]interface{}, + page, limit *int, + filter, orderby, select_ *string, + args ...map[string]interface{}, ) (*vmmv4.ListImagesApiResponse, error) { - return m.images, nil + return m.listImagesFunc(page, limit, filter, orderby, select_) } diff --git a/pkg/webhook/preflight/nutanix/image_test.go b/pkg/webhook/preflight/nutanix/image_test.go index 355557150..ee2b9ac86 100644 --- a/pkg/webhook/preflight/nutanix/image_test.go +++ b/pkg/webhook/preflight/nutanix/image_test.go @@ -19,39 +19,6 @@ import ( "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight" ) -// mockV4Client is a mock implementation of the v4client interface for testing. -type mockV4Client struct { - getImageByIdFunc func( - uuid *string, - ) ( - *vmmv4.GetImageApiResponse, error, - ) - - listImagesFunc func( - page, - limit *int, - filter, - orderby, - select_ *string, - args ...map[string]interface{}, - ) ( - *vmmv4.ListImagesApiResponse, - error, - ) -} - -func (m *mockV4Client) GetImageById(uuid *string) (*vmmv4.GetImageApiResponse, error) { - return m.getImageByIdFunc(uuid) -} - -func (m *mockV4Client) ListImages( - page, limit *int, - filter, orderby, select_ *string, - args ...map[string]interface{}, -) (*vmmv4.ListImagesApiResponse, error) { - return m.listImagesFunc(page, limit, filter, orderby, select_) -} - func TestVMImageCheck(t *testing.T) { testCases := []struct { name string @@ -61,7 +28,7 @@ func TestVMImageCheck(t *testing.T) { }{ { name: "imageLookup not yet supported", - v4client: &mockV4Client{}, + v4client: &mockv4client{}, machineDetails: &carenv1.NutanixMachineDetails{ ImageLookup: &capxv1.NutanixImageLookup{ Format: ptr.To("test-format"), @@ -78,7 +45,7 @@ func TestVMImageCheck(t *testing.T) { }, { name: "image found by uuid", - v4client: &mockV4Client{ + v4client: &mockv4client{ getImageByIdFunc: func(uuid *string) (*vmmv4.GetImageApiResponse, error) { assert.Equal(t, "test-uuid", *uuid) resp := &vmmv4.GetImageApiResponse{} @@ -103,7 +70,7 @@ func TestVMImageCheck(t *testing.T) { }, { name: "image found by name", - v4client: &mockV4Client{ + v4client: &mockv4client{ listImagesFunc: func(page, limit *int, filter, @@ -137,7 +104,7 @@ func TestVMImageCheck(t *testing.T) { }, { name: "image not found by name", - v4client: &mockV4Client{ + v4client: &mockv4client{ listImagesFunc: func(page, limit *int, filter, @@ -170,7 +137,7 @@ func TestVMImageCheck(t *testing.T) { }, { name: "multiple images found by name", - v4client: &mockV4Client{ + v4client: &mockv4client{ listImagesFunc: func(page, limit *int, filter, @@ -213,7 +180,7 @@ func TestVMImageCheck(t *testing.T) { }, { name: "error getting image by id", - v4client: &mockV4Client{ + v4client: &mockv4client{ getImageByIdFunc: func(uuid *string) (*vmmv4.GetImageApiResponse, error) { return nil, fmt.Errorf("api error") }, @@ -238,7 +205,7 @@ func TestVMImageCheck(t *testing.T) { }, { name: "error listing images", - v4client: &mockV4Client{ + v4client: &mockv4client{ listImagesFunc: func(page, limit *int, filter, @@ -272,7 +239,7 @@ func TestVMImageCheck(t *testing.T) { }, { name: "neither image nor imageLookup specified", - v4client: &mockV4Client{}, + v4client: &mockv4client{}, machineDetails: &carenv1.NutanixMachineDetails{ // both Image and ImageLookup are nil }, @@ -314,7 +281,7 @@ func TestVMImageCheck(t *testing.T) { func TestGetVMImages(t *testing.T) { testCases := []struct { name string - client *mockV4Client + client *mockv4client id *capxv1.NutanixResourceIdentifier want []vmmv4.Image wantErr bool @@ -322,7 +289,7 @@ func TestGetVMImages(t *testing.T) { }{ { name: "get image by uuid success", - client: &mockV4Client{ + client: &mockv4client{ getImageByIdFunc: func(uuid *string) (*vmmv4.GetImageApiResponse, error) { assert.Equal(t, "test-uuid", *uuid) resp := &vmmv4.GetImageApiResponse{} @@ -348,7 +315,7 @@ func TestGetVMImages(t *testing.T) { }, { name: "get image by name success", - client: &mockV4Client{ + client: &mockv4client{ listImagesFunc: func(page, limit *int, filter, @@ -384,7 +351,7 @@ func TestGetVMImages(t *testing.T) { }, { name: "get image by uuid error", - client: &mockV4Client{ + client: &mockv4client{ getImageByIdFunc: func(uuid *string) (*vmmv4.GetImageApiResponse, error) { return nil, fmt.Errorf("api error") }, @@ -398,7 +365,7 @@ func TestGetVMImages(t *testing.T) { }, { name: "get image by name error", - client: &mockV4Client{ + client: &mockv4client{ listImagesFunc: func(page, limit *int, filter, @@ -421,7 +388,7 @@ func TestGetVMImages(t *testing.T) { }, { name: "neither name nor uuid specified", - client: &mockV4Client{}, + client: &mockv4client{}, id: &capxv1.NutanixResourceIdentifier{ // Both Name and UUID are not set }, @@ -430,7 +397,7 @@ func TestGetVMImages(t *testing.T) { }, { name: "invalid data from GetImageById", - client: &mockV4Client{ + client: &mockv4client{ getImageByIdFunc: func(uuid *string) (*vmmv4.GetImageApiResponse, error) { return &vmmv4.GetImageApiResponse{ Data: &vmmv4.OneOfGetImageApiResponseData{ @@ -448,7 +415,7 @@ func TestGetVMImages(t *testing.T) { }, { name: "empty response from ListImages", - client: &mockV4Client{ + client: &mockv4client{ listImagesFunc: func(page, limit *int, filter, From 05b342e807556cf4759204489ab74c20bd5dd71e Mon Sep 17 00:00:00 2001 From: Daniel Lipovetsky Date: Thu, 12 Jun 2025 09:07:45 -0700 Subject: [PATCH 08/12] fixup! feat: Nutanix VM image preflight check Merge separate v3 and v4 clients into one --- pkg/webhook/preflight/nutanix/checker.go | 10 +-- pkg/webhook/preflight/nutanix/clients.go | 75 ++++++++-------- pkg/webhook/preflight/nutanix/clients_test.go | 54 ++++++++++++ pkg/webhook/preflight/nutanix/credentials.go | 26 ++---- .../preflight/nutanix/credentials_test.go | 85 ++----------------- pkg/webhook/preflight/nutanix/image.go | 6 +- pkg/webhook/preflight/nutanix/image_test.go | 58 ++++++------- 7 files changed, 138 insertions(+), 176 deletions(-) create mode 100644 pkg/webhook/preflight/nutanix/clients_test.go diff --git a/pkg/webhook/preflight/nutanix/checker.go b/pkg/webhook/preflight/nutanix/checker.go index 4d8fdddc1..882eea786 100644 --- a/pkg/webhook/preflight/nutanix/checker.go +++ b/pkg/webhook/preflight/nutanix/checker.go @@ -22,8 +22,7 @@ func New(kclient ctrlclient.Client, cluster *clusterv1.Cluster) preflight.Checke kclient: kclient, cluster: cluster, - v3clientFactory: newV3Client, - v4clientFactory: newV4Client, + nclientFactory: newClient, vmImageCheckFunc: vmImageCheck, initNutanixConfigurationFunc: initNutanixConfiguration, @@ -39,11 +38,8 @@ type nutanixChecker struct { nutanixClusterConfigSpec *carenv1.NutanixClusterConfigSpec nutanixWorkerNodeConfigSpecByMachineDeploymentName map[string]*carenv1.NutanixWorkerNodeConfigSpec - v3client v3client - v3clientFactory func(prismgoclient.Credentials) (v3client, error) - - v4client v4client - v4clientFactory func(prismgoclient.Credentials) (v4client, error) + nclient client + nclientFactory func(prismgoclient.Credentials) (client, error) vmImageCheckFunc func( n *nutanixChecker, diff --git a/pkg/webhook/preflight/nutanix/clients.go b/pkg/webhook/preflight/nutanix/clients.go index cac4a29a1..c80bc111e 100644 --- a/pkg/webhook/preflight/nutanix/clients.go +++ b/pkg/webhook/preflight/nutanix/clients.go @@ -5,6 +5,7 @@ package nutanix import ( "context" + "fmt" vmmv4 "github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4/models/vmm/v4/content" @@ -13,29 +14,10 @@ import ( prismv4 "github.com/nutanix-cloud-native/prism-go-client/v4" ) -type v3client interface { +// client contains methods to interact with Nutanix Prism v3 and v4 APIs. +type client interface { GetCurrentLoggedInUser(ctx context.Context) (*prismv3.UserIntentResponse, error) -} - -type v3clientWrapper struct { - prismv3.Service -} - -var _ = v3client(&v3clientWrapper{}) -func newV3Client( - credentials prismgoclient.Credentials, //nolint:gocritic // hugeParam is fine -) (v3client, error) { - client, err := prismv3.NewV3Client(credentials) - if err != nil { - return nil, err - } - return &v3clientWrapper{ - client.V3, - }, nil -} - -type v4client interface { GetImageById(id *string) (*vmmv4.GetImageApiResponse, error) ListImages(page_ *int, limit_ *int, @@ -49,28 +31,53 @@ type v4client interface { ) } -type v4clientWrapper struct { - client *prismv4.Client +// clientWrapper implements the client interface and wraps both v3 and v4 clients. +type clientWrapper struct { + v3client *prismv3.Client + v4client *prismv4.Client +} + +var _ = client(&clientWrapper{}) + +func newClient( + credentials prismgoclient.Credentials, //nolint:gocritic // hugeParam is fine +) (client, error) { + v3c, err := prismv3.NewV3Client(credentials) + if err != nil { + return nil, fmt.Errorf("failed to create v3 client: %w", err) + } + + v4c, err := prismv4.NewV4Client(credentials) + if err != nil { + return nil, fmt.Errorf("failed to create v4 client: %w", err) + } + + return &clientWrapper{ + v3client: v3c, + v4client: v4c, + }, nil } -var _ = v4client(&v4clientWrapper{}) +func (c *clientWrapper) GetCurrentLoggedInUser(ctx context.Context) (*prismv3.UserIntentResponse, error) { + return c.v3client.V3.GetCurrentLoggedInUser(ctx) +} -func (c *v4clientWrapper) GetImageById(id *string) (*vmmv4.GetImageApiResponse, error) { - resp, err := c.client.ImagesApiInstance.GetImageById(id) +func (c *clientWrapper) GetImageById(id *string) (*vmmv4.GetImageApiResponse, error) { + resp, err := c.v4client.ImagesApiInstance.GetImageById(id) if err != nil { return nil, err } return resp, nil } -func (c *v4clientWrapper) ListImages(page_ *int, +func (c *clientWrapper) ListImages(page_ *int, limit_ *int, filter_ *string, orderby_ *string, select_ *string, args ...map[string]interface{}, ) (*vmmv4.ListImagesApiResponse, error) { - resp, err := c.client.ImagesApiInstance.ListImages( + resp, err := c.v4client.ImagesApiInstance.ListImages( page_, limit_, filter_, @@ -83,15 +90,3 @@ func (c *v4clientWrapper) ListImages(page_ *int, } return resp, nil } - -func newV4Client( - credentials prismgoclient.Credentials, //nolint:gocritic // hugeParam is fine -) (v4client, error) { - client, err := prismv4.NewV4Client(credentials) - if err != nil { - return nil, err - } - return &v4clientWrapper{ - client: client, - }, nil -} diff --git a/pkg/webhook/preflight/nutanix/clients_test.go b/pkg/webhook/preflight/nutanix/clients_test.go new file mode 100644 index 000000000..acf66f5f6 --- /dev/null +++ b/pkg/webhook/preflight/nutanix/clients_test.go @@ -0,0 +1,54 @@ +// Copyright 2024 Nutanix. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package nutanix + +import ( + "context" + + vmmv4 "github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4/models/vmm/v4/content" + + prismv3 "github.com/nutanix-cloud-native/prism-go-client/v3" +) + +var _ = client(&mocknclient{}) + +// mocknclient is a mock implementation of the client interface for testing purposes. +type mocknclient struct { + user *prismv3.UserIntentResponse + err error + + getImageByIdFunc func( + uuid *string, + ) ( + *vmmv4.GetImageApiResponse, error, + ) + + listImagesFunc func( + page, + limit *int, + filter, + orderby, + select_ *string, + args ...map[string]interface{}, + ) ( + *vmmv4.ListImagesApiResponse, + error, + ) +} + +func (m *mocknclient) GetCurrentLoggedInUser(ctx context.Context) (*prismv3.UserIntentResponse, error) { + return m.user, m.err +} + +func (m *mocknclient) GetImageById(uuid *string) (*vmmv4.GetImageApiResponse, error) { + return m.getImageByIdFunc(uuid) +} + +func (m *mocknclient) ListImages( + page, limit *int, + filter, orderby, select_ *string, + args ...map[string]interface{}, +) (*vmmv4.ListImagesApiResponse, error) { + return m.listImagesFunc(page, limit, filter, orderby, select_) +} diff --git a/pkg/webhook/preflight/nutanix/credentials.go b/pkg/webhook/preflight/nutanix/credentials.go index 7122b1ef6..7564038cb 100644 --- a/pkg/webhook/preflight/nutanix/credentials.go +++ b/pkg/webhook/preflight/nutanix/credentials.go @@ -154,39 +154,24 @@ func initCredentialsCheck( Insecure: prismCentralEndpointSpec.Insecure, } - // Initialize the clients. - v4client, err := n.v4clientFactory(credentials) + // Initialize the Nutanix client. + nclient, err := n.nclientFactory(credentials) if err != nil { result.Allowed = false result.Error = true result.Causes = append(result.Causes, preflight.Cause{ - Message: fmt.Sprintf("failed to initialize Nutanix v4 client: %s", err), + Message: fmt.Sprintf("Failed to initialize Nutanix client: %s", err), Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials", }, ) - } - - v3client, err := n.v3clientFactory(credentials) - if err != nil { - result.Allowed = false - result.Error = true - result.Causes = append(result.Causes, - preflight.Cause{ - Message: fmt.Sprintf("failed to initialize Nutanix v3 client: %s", err), - Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials", - }, - ) - } - - if v3client == nil || v4client == nil { return func(ctx context.Context) preflight.CheckResult { return result } } // Validate the credentials using an API call. - _, err = v3client.GetCurrentLoggedInUser(ctx) + _, err = nclient.GetCurrentLoggedInUser(ctx) if err != nil { result.Allowed = false result.Error = true @@ -203,8 +188,7 @@ func initCredentialsCheck( } // We initialized both clients, and verified the credentials using the v3 client. - n.v3client = v3client - n.v4client = v4client + n.nclient = nclient return func(ctx context.Context) preflight.CheckResult { return result diff --git a/pkg/webhook/preflight/nutanix/credentials_test.go b/pkg/webhook/preflight/nutanix/credentials_test.go index 9c0182a84..e3956f477 100644 --- a/pkg/webhook/preflight/nutanix/credentials_test.go +++ b/pkg/webhook/preflight/nutanix/credentials_test.go @@ -7,7 +7,6 @@ import ( "context" "testing" - vmmv4 "github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4/models/vmm/v4/content" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -15,7 +14,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" prismgoclient "github.com/nutanix-cloud-native/prism-go-client" - prismv3 "github.com/nutanix-cloud-native/prism-go-client/v3" carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" ) @@ -129,47 +127,27 @@ func TestInitCredentialsCheck_InvalidCredentialsFormat(t *testing.T) { assert.Contains(t, result.Causes[0].Message, "failed to parse Prism Central credentials") } -func TestInitCredentialsCheck_FailedToCreateV3Client(t *testing.T) { - // Simulate a failure in creating the v3 client - nc := validNutanixChecker() - nc.v3clientFactory = func(_ prismgoclient.Credentials) (v3client, error) { - return nil, assert.AnError - } - nc.v4clientFactory = func(_ prismgoclient.Credentials) (v4client, error) { - return &mockv4client{}, nil - } - check := nc.initCredentialsCheckFunc(context.Background(), nc) - result := check(context.Background()) - assert.False(t, result.Allowed) - assert.True(t, result.Error) - assert.Contains(t, result.Causes[0].Message, "failed to initialize Nutanix v3 client") -} - -func TestInitCredentialsCheck_FailedToCreateV4Client(t *testing.T) { +func TestInitCredentialsCheck_FailedToCreateClient(t *testing.T) { // Simulate a failure in creating the v4 client nc := validNutanixChecker() - nc.v3clientFactory = func(_ prismgoclient.Credentials) (v3client, error) { - return &mockv3client{}, nil - } - nc.v4clientFactory = func(_ prismgoclient.Credentials) (v4client, error) { + nc.nclientFactory = func(_ prismgoclient.Credentials) (client, error) { return nil, assert.AnError } + check := nc.initCredentialsCheckFunc(context.Background(), nc) result := check(context.Background()) assert.False(t, result.Allowed) assert.True(t, result.Error) - assert.Contains(t, result.Causes[0].Message, "failed to initialize Nutanix v4 client") + assert.Contains(t, result.Causes[0].Message, "Failed to initialize Nutanix client") } func TestInitCredentialsCheck_FailedToGetCurrentLoggedInUser(t *testing.T) { // Simulate a failure in getting the current logged-in user nc := validNutanixChecker() - nc.v3clientFactory = func(_ prismgoclient.Credentials) (v3client, error) { - return &mockv3client{err: assert.AnError}, nil - } - nc.v4clientFactory = func(_ prismgoclient.Credentials) (v4client, error) { - return &mockv4client{}, nil + nc.nclientFactory = func(_ prismgoclient.Credentials) (client, error) { + return &mocknclient{err: assert.AnError}, nil } + check := nc.initCredentialsCheckFunc(context.Background(), nc) result := check(context.Background()) assert.False(t, result.Allowed) @@ -207,11 +185,8 @@ func validNutanixChecker() *nutanixChecker { }, }, - v3clientFactory: func(_ prismgoclient.Credentials) (v3client, error) { - return &mockv3client{}, nil - }, - v4clientFactory: func(_ prismgoclient.Credentials) (v4client, error) { - return &mockv4client{}, nil + nclientFactory: func(_ prismgoclient.Credentials) (client, error) { + return &mocknclient{}, nil }, vmImageCheckFunc: vmImageCheck, @@ -233,45 +208,3 @@ func validNutanixChecker() *nutanixChecker { nutanixWorkerNodeConfigSpecByMachineDeploymentName: map[string]*carenv1.NutanixWorkerNodeConfigSpec{}, } } - -type mockv3client struct { - user *prismv3.UserIntentResponse - err error -} - -func (m *mockv3client) GetCurrentLoggedInUser(ctx context.Context) (*prismv3.UserIntentResponse, error) { - return m.user, m.err -} - -// mockv4client is a mock implementation of the v4client interface for testing. -type mockv4client struct { - getImageByIdFunc func( - uuid *string, - ) ( - *vmmv4.GetImageApiResponse, error, - ) - - listImagesFunc func( - page, - limit *int, - filter, - orderby, - select_ *string, - args ...map[string]interface{}, - ) ( - *vmmv4.ListImagesApiResponse, - error, - ) -} - -func (m *mockv4client) GetImageById(uuid *string) (*vmmv4.GetImageApiResponse, error) { - return m.getImageByIdFunc(uuid) -} - -func (m *mockv4client) ListImages( - page, limit *int, - filter, orderby, select_ *string, - args ...map[string]interface{}, -) (*vmmv4.ListImagesApiResponse, error) { - return m.listImagesFunc(page, limit, filter, orderby, select_) -} diff --git a/pkg/webhook/preflight/nutanix/image.go b/pkg/webhook/preflight/nutanix/image.go index 9eadca2f1..c628d0922 100644 --- a/pkg/webhook/preflight/nutanix/image.go +++ b/pkg/webhook/preflight/nutanix/image.go @@ -19,7 +19,7 @@ func initVMImageChecks( ) []preflight.Check { checks := []preflight.Check{} - if n.v4client == nil { + if n.nclient == nil { return checks } @@ -81,7 +81,7 @@ func vmImageCheck( errCh := make(chan error) defer close(errCh) - images, err := getVMImages(n.v4client, machineDetails.Image) + images, err := getVMImages(n.nclient, machineDetails.Image) if err != nil { result.Allowed = false result.Error = true @@ -112,7 +112,7 @@ func vmImageCheck( } func getVMImages( - client v4client, + client client, id *capxv1.NutanixResourceIdentifier, ) ([]vmmv4.Image, error) { switch { diff --git a/pkg/webhook/preflight/nutanix/image_test.go b/pkg/webhook/preflight/nutanix/image_test.go index ee2b9ac86..aa1f4b4bb 100644 --- a/pkg/webhook/preflight/nutanix/image_test.go +++ b/pkg/webhook/preflight/nutanix/image_test.go @@ -22,13 +22,13 @@ import ( func TestVMImageCheck(t *testing.T) { testCases := []struct { name string - v4client v4client + nclient client machineDetails *carenv1.NutanixMachineDetails want preflight.CheckResult }{ { - name: "imageLookup not yet supported", - v4client: &mockv4client{}, + name: "imageLookup not yet supported", + nclient: &mocknclient{}, machineDetails: &carenv1.NutanixMachineDetails{ ImageLookup: &capxv1.NutanixImageLookup{ Format: ptr.To("test-format"), @@ -45,7 +45,7 @@ func TestVMImageCheck(t *testing.T) { }, { name: "image found by uuid", - v4client: &mockv4client{ + nclient: &mocknclient{ getImageByIdFunc: func(uuid *string) (*vmmv4.GetImageApiResponse, error) { assert.Equal(t, "test-uuid", *uuid) resp := &vmmv4.GetImageApiResponse{} @@ -70,7 +70,7 @@ func TestVMImageCheck(t *testing.T) { }, { name: "image found by name", - v4client: &mockv4client{ + nclient: &mocknclient{ listImagesFunc: func(page, limit *int, filter, @@ -104,7 +104,7 @@ func TestVMImageCheck(t *testing.T) { }, { name: "image not found by name", - v4client: &mockv4client{ + nclient: &mocknclient{ listImagesFunc: func(page, limit *int, filter, @@ -137,7 +137,7 @@ func TestVMImageCheck(t *testing.T) { }, { name: "multiple images found by name", - v4client: &mockv4client{ + nclient: &mocknclient{ listImagesFunc: func(page, limit *int, filter, @@ -180,7 +180,7 @@ func TestVMImageCheck(t *testing.T) { }, { name: "error getting image by id", - v4client: &mockv4client{ + nclient: &mocknclient{ getImageByIdFunc: func(uuid *string) (*vmmv4.GetImageApiResponse, error) { return nil, fmt.Errorf("api error") }, @@ -205,7 +205,7 @@ func TestVMImageCheck(t *testing.T) { }, { name: "error listing images", - v4client: &mockv4client{ + nclient: &mocknclient{ listImagesFunc: func(page, limit *int, filter, @@ -239,7 +239,7 @@ func TestVMImageCheck(t *testing.T) { }, { name: "neither image nor imageLookup specified", - v4client: &mockv4client{}, + nclient: &mocknclient{}, machineDetails: &carenv1.NutanixMachineDetails{ // both Image and ImageLookup are nil }, @@ -255,8 +255,8 @@ func TestVMImageCheck(t *testing.T) { logger := testr.New(t) checker := &nutanixChecker{ - log: logger, - v4client: tc.v4client, + log: logger, + nclient: tc.nclient, } // Create the check @@ -281,7 +281,7 @@ func TestVMImageCheck(t *testing.T) { func TestGetVMImages(t *testing.T) { testCases := []struct { name string - client *mockv4client + client *mocknclient id *capxv1.NutanixResourceIdentifier want []vmmv4.Image wantErr bool @@ -289,7 +289,7 @@ func TestGetVMImages(t *testing.T) { }{ { name: "get image by uuid success", - client: &mockv4client{ + client: &mocknclient{ getImageByIdFunc: func(uuid *string) (*vmmv4.GetImageApiResponse, error) { assert.Equal(t, "test-uuid", *uuid) resp := &vmmv4.GetImageApiResponse{} @@ -315,7 +315,7 @@ func TestGetVMImages(t *testing.T) { }, { name: "get image by name success", - client: &mockv4client{ + client: &mocknclient{ listImagesFunc: func(page, limit *int, filter, @@ -351,7 +351,7 @@ func TestGetVMImages(t *testing.T) { }, { name: "get image by uuid error", - client: &mockv4client{ + client: &mocknclient{ getImageByIdFunc: func(uuid *string) (*vmmv4.GetImageApiResponse, error) { return nil, fmt.Errorf("api error") }, @@ -365,7 +365,7 @@ func TestGetVMImages(t *testing.T) { }, { name: "get image by name error", - client: &mockv4client{ + client: &mocknclient{ listImagesFunc: func(page, limit *int, filter, @@ -388,7 +388,7 @@ func TestGetVMImages(t *testing.T) { }, { name: "neither name nor uuid specified", - client: &mockv4client{}, + client: &mocknclient{}, id: &capxv1.NutanixResourceIdentifier{ // Both Name and UUID are not set }, @@ -397,7 +397,7 @@ func TestGetVMImages(t *testing.T) { }, { name: "invalid data from GetImageById", - client: &mockv4client{ + client: &mocknclient{ getImageByIdFunc: func(uuid *string) (*vmmv4.GetImageApiResponse, error) { return &vmmv4.GetImageApiResponse{ Data: &vmmv4.OneOfGetImageApiResponseData{ @@ -415,7 +415,7 @@ func TestGetVMImages(t *testing.T) { }, { name: "empty response from ListImages", - client: &mockv4client{ + client: &mocknclient{ listImagesFunc: func(page, limit *int, filter, @@ -460,7 +460,7 @@ func TestInitVMImageChecks(t *testing.T) { name string nutanixClusterConfigSpec *carenv1.NutanixClusterConfigSpec nutanixWorkerNodeConfigSpecByMDName map[string]*carenv1.NutanixWorkerNodeConfigSpec - v4client v4client + nclient client expectedChecks int expectedControlPlaneCheckFieldIncluded bool expectedWorkerNodeCheckFieldPatternExists bool @@ -469,7 +469,7 @@ func TestInitVMImageChecks(t *testing.T) { name: "client not initialized", nutanixClusterConfigSpec: nil, nutanixWorkerNodeConfigSpecByMDName: nil, - v4client: nil, + nclient: nil, expectedChecks: 0, expectedControlPlaneCheckFieldIncluded: false, expectedWorkerNodeCheckFieldPatternExists: false, @@ -478,7 +478,7 @@ func TestInitVMImageChecks(t *testing.T) { name: "no nutanix configuration", nutanixClusterConfigSpec: nil, nutanixWorkerNodeConfigSpecByMDName: nil, - v4client: &mockv4client{}, + nclient: &mocknclient{}, expectedChecks: 0, expectedControlPlaneCheckFieldIncluded: false, expectedWorkerNodeCheckFieldPatternExists: false, @@ -498,7 +498,7 @@ func TestInitVMImageChecks(t *testing.T) { }, }, nutanixWorkerNodeConfigSpecByMDName: nil, - v4client: &mockv4client{}, + nclient: &mocknclient{}, expectedChecks: 1, expectedControlPlaneCheckFieldIncluded: true, expectedWorkerNodeCheckFieldPatternExists: false, @@ -518,7 +518,7 @@ func TestInitVMImageChecks(t *testing.T) { }, }, }, - v4client: &mockv4client{}, + nclient: &mocknclient{}, expectedChecks: 1, expectedControlPlaneCheckFieldIncluded: false, expectedWorkerNodeCheckFieldPatternExists: true, @@ -559,7 +559,7 @@ func TestInitVMImageChecks(t *testing.T) { }, }, }, - v4client: &mockv4client{}, + nclient: &mocknclient{}, expectedChecks: 3, // 1 control plane + 2 workers expectedControlPlaneCheckFieldIncluded: true, expectedWorkerNodeCheckFieldPatternExists: true, @@ -582,7 +582,7 @@ func TestInitVMImageChecks(t *testing.T) { }, }, }, - v4client: &mockv4client{}, + nclient: &mocknclient{}, expectedChecks: 1, // only worker-2 expectedControlPlaneCheckFieldIncluded: false, expectedWorkerNodeCheckFieldPatternExists: true, @@ -605,7 +605,7 @@ func TestInitVMImageChecks(t *testing.T) { ControlPlane: nil, // null control plane }, nutanixWorkerNodeConfigSpecByMDName: nil, - v4client: &mockv4client{}, + nclient: &mocknclient{}, expectedChecks: 0, expectedControlPlaneCheckFieldIncluded: false, expectedWorkerNodeCheckFieldPatternExists: false, @@ -619,7 +619,7 @@ func TestInitVMImageChecks(t *testing.T) { log: logger, nutanixClusterConfigSpec: tc.nutanixClusterConfigSpec, nutanixWorkerNodeConfigSpecByMachineDeploymentName: tc.nutanixWorkerNodeConfigSpecByMDName, - v4client: tc.v4client, + nclient: tc.nclient, } // Trap the vmImageCheck calls to verify field paths From 19ab822a586abb098d2c24dc536866eff3c89697 Mon Sep 17 00:00:00 2001 From: Daniel Lipovetsky Date: Thu, 12 Jun 2025 09:08:21 -0700 Subject: [PATCH 09/12] fixup! feat: Nutanix VM image preflight check Remove unused code --- pkg/webhook/preflight/nutanix/image.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pkg/webhook/preflight/nutanix/image.go b/pkg/webhook/preflight/nutanix/image.go index c628d0922..3dec08305 100644 --- a/pkg/webhook/preflight/nutanix/image.go +++ b/pkg/webhook/preflight/nutanix/image.go @@ -76,11 +76,6 @@ func vmImageCheck( } if machineDetails.Image != nil { - imagesCh := make(chan []vmmv4.Image) - defer close(imagesCh) - errCh := make(chan error) - defer close(errCh) - images, err := getVMImages(n.nclient, machineDetails.Image) if err != nil { result.Allowed = false From f567dfeba1cbf337412bac680232313287c8e0ed Mon Sep 17 00:00:00 2001 From: Daniel Lipovetsky Date: Fri, 13 Jun 2025 08:42:02 -0700 Subject: [PATCH 10/12] fixup! feat: Nutanix VM image preflight check Implement Check interface. --- pkg/webhook/preflight/nutanix/checker.go | 7 - pkg/webhook/preflight/nutanix/checker_test.go | 46 ++++-- pkg/webhook/preflight/nutanix/credentials.go | 107 ++++++------- .../preflight/nutanix/credentials_test.go | 21 ++- pkg/webhook/preflight/nutanix/image.go | 133 ++++++++-------- pkg/webhook/preflight/nutanix/image_test.go | 146 +++++++++--------- pkg/webhook/preflight/nutanix/specs.go | 35 +++-- pkg/webhook/preflight/nutanix/specs_test.go | 13 +- 8 files changed, 253 insertions(+), 255 deletions(-) diff --git a/pkg/webhook/preflight/nutanix/checker.go b/pkg/webhook/preflight/nutanix/checker.go index 882eea786..bcae1fe5e 100644 --- a/pkg/webhook/preflight/nutanix/checker.go +++ b/pkg/webhook/preflight/nutanix/checker.go @@ -24,7 +24,6 @@ func New(kclient ctrlclient.Client, cluster *clusterv1.Cluster) preflight.Checke nclientFactory: newClient, - vmImageCheckFunc: vmImageCheck, initNutanixConfigurationFunc: initNutanixConfiguration, initCredentialsCheckFunc: initCredentialsCheck, initVMImageChecksFunc: initVMImageChecks, @@ -41,12 +40,6 @@ type nutanixChecker struct { nclient client nclientFactory func(prismgoclient.Credentials) (client, error) - vmImageCheckFunc func( - n *nutanixChecker, - machineDetails *carenv1.NutanixMachineDetails, - field string, - ) preflight.Check - initNutanixConfigurationFunc func( n *nutanixChecker, ) preflight.Check diff --git a/pkg/webhook/preflight/nutanix/checker_test.go b/pkg/webhook/preflight/nutanix/checker_test.go index 93f054207..8c6579d76 100644 --- a/pkg/webhook/preflight/nutanix/checker_test.go +++ b/pkg/webhook/preflight/nutanix/checker_test.go @@ -15,6 +15,19 @@ import ( "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight" ) +type mockCheck struct { + name string + result preflight.CheckResult +} + +func (m *mockCheck) Name() string { + return m.name +} + +func (m *mockCheck) Run(ctx context.Context) preflight.CheckResult { + return m.result +} + func TestNutanixChecker_Init(t *testing.T) { tests := []struct { name string @@ -98,19 +111,17 @@ func TestNutanixChecker_Init(t *testing.T) { checker.initNutanixConfigurationFunc = func(n *nutanixChecker) preflight.Check { configCheckCalled = true - return func(ctx context.Context) preflight.CheckResult { - return preflight.CheckResult{ - Name: tt.expectedFirstCheckName, - } + return &mockCheck{ + name: tt.expectedFirstCheckName, + result: preflight.CheckResult{Allowed: true}, } } checker.initCredentialsCheckFunc = func(ctx context.Context, n *nutanixChecker) preflight.Check { credsCheckCalled = true - return func(ctx context.Context) preflight.CheckResult { - return preflight.CheckResult{ - Name: tt.expectedSecondCheckName, - } + return &mockCheck{ + name: tt.expectedSecondCheckName, + result: preflight.CheckResult{Allowed: true}, } } @@ -118,11 +129,14 @@ func TestNutanixChecker_Init(t *testing.T) { checks := []preflight.Check{} for i := 0; i < tt.vmImageCheckCount; i++ { vmImageCheckCount++ - checks = append(checks, func(ctx context.Context) preflight.CheckResult { - return preflight.CheckResult{ - Name: fmt.Sprintf("VMImageCheck-%d", i), - } - }) + checks = append(checks, + &mockCheck{ + name: fmt.Sprintf("NutanixVMImage-%d", i), + result: preflight.CheckResult{ + Allowed: true, + }, + }, + ) } return checks } @@ -144,10 +158,8 @@ func TestNutanixChecker_Init(t *testing.T) { // Verify the first two checks when we have results if len(checks) >= 2 { - result1 := checks[0](ctx) - result2 := checks[1](ctx) - assert.Equal(t, tt.expectedFirstCheckName, result1.Name) - assert.Equal(t, tt.expectedSecondCheckName, result2.Name) + assert.Equal(t, tt.expectedFirstCheckName, checks[0].Name()) + assert.Equal(t, tt.expectedSecondCheckName, checks[1].Name()) } }) } diff --git a/pkg/webhook/preflight/nutanix/credentials.go b/pkg/webhook/preflight/nutanix/credentials.go index 7564038cb..30c2d88b0 100644 --- a/pkg/webhook/preflight/nutanix/credentials.go +++ b/pkg/webhook/preflight/nutanix/credentials.go @@ -18,38 +18,47 @@ import ( const credentialsSecretDataKey = "credentials" +type credentialsCheck struct { + result preflight.CheckResult +} + +func (c *credentialsCheck) Name() string { + return "NutanixCredentials" +} + +func (c *credentialsCheck) Run(_ context.Context) preflight.CheckResult { + return c.result +} + func initCredentialsCheck( ctx context.Context, n *nutanixChecker, ) preflight.Check { n.log.V(5).Info("Initializing Nutanix credentials check") - result := preflight.CheckResult{ - Name: "NutanixCredentials", - Allowed: true, + credentialsCheck := &credentialsCheck{ + result: preflight.CheckResult{ + Allowed: true, + }, } if n.nutanixClusterConfigSpec == nil && len(n.nutanixWorkerNodeConfigSpecByMachineDeploymentName) == 0 { // If there is no Nutanix configuration at all, the credentials check is not needed. - return func(ctx context.Context) preflight.CheckResult { - return result - } + return credentialsCheck } // There is some Nutanix configuration, so the credentials check is needed. // However, the credentials configuration is missing, so we cannot perform the check. if n.nutanixClusterConfigSpec == nil || n.nutanixClusterConfigSpec.Nutanix == nil { - result.Allowed = false - result.Error = true - result.Causes = append(result.Causes, + credentialsCheck.result.Allowed = false + credentialsCheck.result.Error = true + credentialsCheck.result.Causes = append(credentialsCheck.result.Causes, preflight.Cause{ Message: "Nutanix cluster configuration is not defined in the cluster spec", Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix", }, ) - return func(ctx context.Context) preflight.CheckResult { - return result - } + return credentialsCheck } // Get the credentials data in order to initialize the credentials and clients. @@ -58,17 +67,15 @@ func initCredentialsCheck( host, port, err := prismCentralEndpointSpec.ParseURL() if err != nil { // Should not happen if the cluster passed CEL validation rules. - result.Allowed = false - result.Error = true - result.Causes = append(result.Causes, + credentialsCheck.result.Allowed = false + credentialsCheck.result.Error = true + credentialsCheck.result.Causes = append(credentialsCheck.result.Causes, preflight.Cause{ Message: fmt.Sprintf("failed to parse Prism Central endpoint URL: %s", err), Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.url", }, ) - return func(ctx context.Context) preflight.CheckResult { - return result - } + return credentialsCheck } credentialsSecret := &corev1.Secret{} @@ -81,23 +88,21 @@ func initCredentialsCheck( credentialsSecret, ) if err != nil { - result.Allowed = false - result.Error = true - result.Causes = append(result.Causes, + credentialsCheck.result.Allowed = false + credentialsCheck.result.Error = true + credentialsCheck.result.Causes = append(credentialsCheck.result.Causes, preflight.Cause{ Message: fmt.Sprintf("failed to get Prism Central credentials Secret: %s", err), Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials.secretRef", }, ) - return func(ctx context.Context) preflight.CheckResult { - return result - } + return credentialsCheck } if len(credentialsSecret.Data) == 0 { - result.Allowed = false - result.Error = true - result.Causes = append(result.Causes, + credentialsCheck.result.Allowed = false + credentialsCheck.result.Error = true + credentialsCheck.result.Causes = append(credentialsCheck.result.Causes, preflight.Cause{ Message: fmt.Sprintf( "credentials Secret '%s' is empty", @@ -106,16 +111,14 @@ func initCredentialsCheck( Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials.secretRef", }, ) - return func(ctx context.Context) preflight.CheckResult { - return result - } + return credentialsCheck } data, ok := credentialsSecret.Data[credentialsSecretDataKey] if !ok { - result.Allowed = false - result.Error = true - result.Causes = append(result.Causes, + credentialsCheck.result.Allowed = false + credentialsCheck.result.Error = true + credentialsCheck.result.Causes = append(credentialsCheck.result.Causes, preflight.Cause{ Message: fmt.Sprintf( "credentials Secret '%s' does not contain key '%s'", @@ -125,24 +128,20 @@ func initCredentialsCheck( Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials.secretRef", }, ) - return func(ctx context.Context) preflight.CheckResult { - return result - } + return credentialsCheck } usernamePassword, err := prismcredentials.ParseCredentials(data) if err != nil { - result.Allowed = false - result.Error = true - result.Causes = append(result.Causes, + credentialsCheck.result.Allowed = false + credentialsCheck.result.Error = true + credentialsCheck.result.Causes = append(credentialsCheck.result.Causes, preflight.Cause{ Message: fmt.Sprintf("failed to parse Prism Central credentials: %s", err), Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials", }, ) - return func(ctx context.Context) preflight.CheckResult { - return result - } + return credentialsCheck } // Initialize the credentials. @@ -157,40 +156,34 @@ func initCredentialsCheck( // Initialize the Nutanix client. nclient, err := n.nclientFactory(credentials) if err != nil { - result.Allowed = false - result.Error = true - result.Causes = append(result.Causes, + credentialsCheck.result.Allowed = false + credentialsCheck.result.Error = true + credentialsCheck.result.Causes = append(credentialsCheck.result.Causes, preflight.Cause{ Message: fmt.Sprintf("Failed to initialize Nutanix client: %s", err), Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials", }, ) - return func(ctx context.Context) preflight.CheckResult { - return result - } + return credentialsCheck } // Validate the credentials using an API call. _, err = nclient.GetCurrentLoggedInUser(ctx) if err != nil { - result.Allowed = false - result.Error = true - result.Causes = append(result.Causes, + credentialsCheck.result.Allowed = false + credentialsCheck.result.Error = true + credentialsCheck.result.Causes = append(credentialsCheck.result.Causes, preflight.Cause{ Message: fmt.Sprintf("Failed to validate credentials using the v3 API client. "+ "The URL and/or credentials may be incorrect. (Error: %q)", err), Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint", }, ) - return func(ctx context.Context) preflight.CheckResult { - return result - } + return credentialsCheck } // We initialized both clients, and verified the credentials using the v3 client. n.nclient = nclient - return func(ctx context.Context) preflight.CheckResult { - return result - } + return credentialsCheck } diff --git a/pkg/webhook/preflight/nutanix/credentials_test.go b/pkg/webhook/preflight/nutanix/credentials_test.go index e3956f477..ac5239209 100644 --- a/pkg/webhook/preflight/nutanix/credentials_test.go +++ b/pkg/webhook/preflight/nutanix/credentials_test.go @@ -21,7 +21,7 @@ import ( func TestInitCredentialsCheck_Success(t *testing.T) { nc := validNutanixChecker() check := nc.initCredentialsCheckFunc(context.Background(), nc) - result := check(context.Background()) + result := check.Run(context.Background()) assert.True(t, result.Allowed) assert.False(t, result.Error) assert.Empty(t, result.Causes) @@ -32,7 +32,7 @@ func TestInitCredentialsCheck_NoNutanixConfig(t *testing.T) { nc.nutanixClusterConfigSpec = nil nc.nutanixWorkerNodeConfigSpecByMachineDeploymentName = map[string]*carenv1.NutanixWorkerNodeConfigSpec{} check := nc.initCredentialsCheckFunc(context.Background(), nc) - result := check(context.Background()) + result := check.Run(context.Background()) assert.True(t, result.Allowed) assert.False(t, result.Error) assert.Empty(t, result.Causes) @@ -42,7 +42,7 @@ func TestInitCredentialsCheck_MissingNutanixField(t *testing.T) { nc := validNutanixChecker() nc.nutanixClusterConfigSpec.Nutanix = nil check := nc.initCredentialsCheckFunc(context.Background(), nc) - result := check(context.Background()) + result := check.Run(context.Background()) assert.False(t, result.Allowed) assert.True(t, result.Error) assert.NotEmpty(t, result.Causes) @@ -53,7 +53,7 @@ func TestInitCredentialsCheck_InvalidURL(t *testing.T) { nc := validNutanixChecker() nc.nutanixClusterConfigSpec.Nutanix.PrismCentralEndpoint.URL = "not-a-url" check := nc.initCredentialsCheckFunc(context.Background(), nc) - result := check(context.Background()) + result := check.Run(context.Background()) assert.False(t, result.Allowed) assert.True(t, result.Error) assert.Contains(t, result.Causes[0].Message, "failed to parse Prism Central endpoint URL") @@ -63,7 +63,7 @@ func TestInitCredentialsCheck_SecretNotFound(t *testing.T) { nc := validNutanixChecker() nc.kclient = fake.NewClientBuilder().Build() // no secret check := nc.initCredentialsCheckFunc(context.Background(), nc) - result := check(context.Background()) + result := check.Run(context.Background()) assert.False(t, result.Allowed) assert.True(t, result.Error) assert.Contains(t, result.Causes[0].Message, "failed to get Prism Central credentials Secret") @@ -81,7 +81,7 @@ func TestInitCredentialsCheck_SecretEmpty(t *testing.T) { nc := validNutanixChecker() nc.kclient = kclient check := nc.initCredentialsCheckFunc(context.Background(), nc) - result := check(context.Background()) + result := check.Run(context.Background()) assert.False(t, result.Allowed) assert.True(t, result.Error) assert.Contains(t, result.Causes[0].Message, "credentials Secret 'ntnx-creds' is empty") @@ -101,7 +101,7 @@ func TestInitCredentialsCheck_SecretMissingKey(t *testing.T) { nc := validNutanixChecker() nc.kclient = kclient check := nc.initCredentialsCheckFunc(context.Background(), nc) - result := check(context.Background()) + result := check.Run(context.Background()) assert.False(t, result.Allowed) assert.True(t, result.Error) assert.Contains(t, result.Causes[0].Message, "does not contain key 'credentials'") @@ -121,7 +121,7 @@ func TestInitCredentialsCheck_InvalidCredentialsFormat(t *testing.T) { nc := validNutanixChecker() nc.kclient = kclient check := nc.initCredentialsCheckFunc(context.Background(), nc) - result := check(context.Background()) + result := check.Run(context.Background()) assert.False(t, result.Allowed) assert.True(t, result.Error) assert.Contains(t, result.Causes[0].Message, "failed to parse Prism Central credentials") @@ -135,7 +135,7 @@ func TestInitCredentialsCheck_FailedToCreateClient(t *testing.T) { } check := nc.initCredentialsCheckFunc(context.Background(), nc) - result := check(context.Background()) + result := check.Run(context.Background()) assert.False(t, result.Allowed) assert.True(t, result.Error) assert.Contains(t, result.Causes[0].Message, "Failed to initialize Nutanix client") @@ -149,7 +149,7 @@ func TestInitCredentialsCheck_FailedToGetCurrentLoggedInUser(t *testing.T) { } check := nc.initCredentialsCheckFunc(context.Background(), nc) - result := check(context.Background()) + result := check.Run(context.Background()) assert.False(t, result.Allowed) assert.True(t, result.Error) assert.Contains(t, result.Causes[0].Message, "Failed to validate credentials using the v3 API client.") @@ -189,7 +189,6 @@ func validNutanixChecker() *nutanixChecker { return &mocknclient{}, nil }, - vmImageCheckFunc: vmImageCheck, initNutanixConfigurationFunc: initNutanixConfiguration, initCredentialsCheckFunc: initCredentialsCheck, diff --git a/pkg/webhook/preflight/nutanix/image.go b/pkg/webhook/preflight/nutanix/image.go index 3dec08305..9224d4a57 100644 --- a/pkg/webhook/preflight/nutanix/image.go +++ b/pkg/webhook/preflight/nutanix/image.go @@ -14,6 +14,60 @@ import ( "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight" ) +type imageCheck struct { + machineDetails *carenv1.NutanixMachineDetails + field string + n *nutanixChecker +} + +func (c *imageCheck) Name() string { + return "NutanixVMImage" +} + +func (c *imageCheck) Run(ctx context.Context) preflight.CheckResult { + result := preflight.CheckResult{ + Allowed: false, + } + + if c.machineDetails.ImageLookup != nil { + result.Allowed = true + result.Warnings = append( + result.Warnings, + fmt.Sprintf("%s uses imageLookup, which is not yet supported by checks", c.field), + ) + return result + } + + if c.machineDetails.Image != nil { + images, err := getVMImages(c.n.nclient, c.machineDetails.Image) + if err != nil { + result.Allowed = false + result.Error = true + result.Causes = append(result.Causes, preflight.Cause{ + Message: fmt.Sprintf("failed to get VM Image: %s", err), + Field: c.field, + }) + return result + } + + if len(images) != 1 { + result.Allowed = false + result.Causes = append(result.Causes, preflight.Cause{ + Message: fmt.Sprintf("expected to find 1 VM Image, found %d", len(images)), + Field: c.field, + }) + return result + } + + // Found exactly one image. + result.Allowed = true + return result + } + + // Neither ImageLookup nor Image is specified. + return result +} + func initVMImageChecks( n *nutanixChecker, ) []preflight.Check { @@ -26,26 +80,24 @@ func initVMImageChecks( if n.nutanixClusterConfigSpec != nil && n.nutanixClusterConfigSpec.ControlPlane != nil && n.nutanixClusterConfigSpec.ControlPlane.Nutanix != nil { checks = append(checks, - n.vmImageCheckFunc( - n, - &n.nutanixClusterConfigSpec.ControlPlane.Nutanix.MachineDetails, - "cluster.spec.topology[.name=clusterConfig].value.controlPlane.nutanix.machineDetails", - ), + &imageCheck{ + machineDetails: &n.nutanixClusterConfigSpec.ControlPlane.Nutanix.MachineDetails, + field: "cluster.spec.topology.variables[.name=clusterConfig]" + + ".value.nutanix.controlPlane.machineDetails", + n: n, + }, ) } for mdName, nutanixWorkerNodeConfigSpec := range n.nutanixWorkerNodeConfigSpecByMachineDeploymentName { if nutanixWorkerNodeConfigSpec.Nutanix != nil { checks = append(checks, - n.vmImageCheckFunc( - n, - &nutanixWorkerNodeConfigSpec.Nutanix.MachineDetails, - fmt.Sprintf( - "cluster.spec.topology.workers.machineDeployments[.name=%s]"+ - ".variables[.name=workerConfig].value.nutanix.machineDetails", - mdName, - ), - ), + &imageCheck{ + machineDetails: &nutanixWorkerNodeConfigSpec.Nutanix.MachineDetails, + field: fmt.Sprintf("cluster.spec.topology.workers.machineDeployments[.name=%s]"+ + ".variables[.name=workerConfig].value.nutanix.machineDetails", mdName), + n: n, + }, ) } } @@ -53,59 +105,6 @@ func initVMImageChecks( return checks } -func vmImageCheck( - n *nutanixChecker, - machineDetails *carenv1.NutanixMachineDetails, - field string, -) preflight.Check { - n.log.V(5).Info("Initializing Nutanix VM image check", "field", field) - - return func(ctx context.Context) preflight.CheckResult { - result := preflight.CheckResult{ - Name: "NutanixVMImage", - Allowed: false, - } - - if machineDetails.ImageLookup != nil { - result.Allowed = true - result.Warnings = append( - result.Warnings, - fmt.Sprintf("%s uses imageLookup, which is not yet supported by checks", field), - ) - return result - } - - if machineDetails.Image != nil { - images, err := getVMImages(n.nclient, machineDetails.Image) - if err != nil { - result.Allowed = false - result.Error = true - result.Causes = append(result.Causes, preflight.Cause{ - Message: fmt.Sprintf("failed to get VM Image: %s", err), - Field: field, - }) - return result - } - - if len(images) != 1 { - result.Allowed = false - result.Causes = append(result.Causes, preflight.Cause{ - Message: fmt.Sprintf("expected to find 1 VM Image, found %d", len(images)), - Field: field, - }) - return result - } - - // Found exactly one image. - result.Allowed = true - return result - } - - // Neither ImageLookup nor Image is specified. - return result - } -} - func getVMImages( client client, id *capxv1.NutanixResourceIdentifier, diff --git a/pkg/webhook/preflight/nutanix/image_test.go b/pkg/webhook/preflight/nutanix/image_test.go index aa1f4b4bb..09b9f2ae4 100644 --- a/pkg/webhook/preflight/nutanix/image_test.go +++ b/pkg/webhook/preflight/nutanix/image_test.go @@ -36,7 +36,6 @@ func TestVMImageCheck(t *testing.T) { }, }, want: preflight.CheckResult{ - Name: "NutanixVMImage", Allowed: true, Warnings: []string{ "test-field uses imageLookup, which is not yet supported by checks", @@ -64,7 +63,6 @@ func TestVMImageCheck(t *testing.T) { }, }, want: preflight.CheckResult{ - Name: "NutanixVMImage", Allowed: true, }, }, @@ -98,7 +96,6 @@ func TestVMImageCheck(t *testing.T) { }, }, want: preflight.CheckResult{ - Name: "NutanixVMImage", Allowed: true, }, }, @@ -125,7 +122,6 @@ func TestVMImageCheck(t *testing.T) { }, }, want: preflight.CheckResult{ - Name: "NutanixVMImage", Allowed: false, Causes: []preflight.Cause{ { @@ -168,7 +164,6 @@ func TestVMImageCheck(t *testing.T) { }, }, want: preflight.CheckResult{ - Name: "NutanixVMImage", Allowed: false, Causes: []preflight.Cause{ { @@ -192,7 +187,6 @@ func TestVMImageCheck(t *testing.T) { }, }, want: preflight.CheckResult{ - Name: "NutanixVMImage", Allowed: false, Error: true, Causes: []preflight.Cause{ @@ -226,7 +220,6 @@ func TestVMImageCheck(t *testing.T) { }, }, want: preflight.CheckResult{ - Name: "NutanixVMImage", Allowed: false, Error: true, Causes: []preflight.Cause{ @@ -244,7 +237,6 @@ func TestVMImageCheck(t *testing.T) { // both Image and ImageLookup are nil }, want: preflight.CheckResult{ - Name: "NutanixVMImage", Allowed: false, }, }, @@ -260,17 +252,16 @@ func TestVMImageCheck(t *testing.T) { } // Create the check - checkFn := vmImageCheck( - checker, - tc.machineDetails, - "test-field", - ) + check := &imageCheck{ + machineDetails: tc.machineDetails, + field: "test-field", + n: checker, + } // Execute the check - got := checkFn(context.Background()) + got := check.Run(context.Background()) // Verify the result - assert.Equal(t, tc.want.Name, got.Name) assert.Equal(t, tc.want.Allowed, got.Allowed) assert.Equal(t, tc.want.Error, got.Error) assert.Equal(t, tc.want.Causes, got.Causes) @@ -497,8 +488,19 @@ func TestInitVMImageChecks(t *testing.T) { }, }, }, - nutanixWorkerNodeConfigSpecByMDName: nil, - nclient: &mocknclient{}, + nutanixWorkerNodeConfigSpecByMDName: nil, + nclient: &mocknclient{ + getImageByIdFunc: func(uuid *string) (*vmmv4.GetImageApiResponse, error) { + assert.Equal(t, "test-uuid", *uuid) + resp := &vmmv4.GetImageApiResponse{} + err := resp.SetData(vmmv4.Image{ + ObjectType_: ptr.To("vmm.v4.content.Image"), + ExtId: ptr.To("test-uuid"), + }) + require.NoError(t, err) + return resp, nil + }, + }, expectedChecks: 1, expectedControlPlaneCheckFieldIncluded: true, expectedWorkerNodeCheckFieldPatternExists: false, @@ -511,16 +513,27 @@ func TestInitVMImageChecks(t *testing.T) { Nutanix: &carenv1.NutanixNodeSpec{ MachineDetails: carenv1.NutanixMachineDetails{ Image: &capxv1.NutanixResourceIdentifier{ - Type: capxv1.NutanixIdentifierName, - Name: ptr.To("worker-image"), + Type: capxv1.NutanixIdentifierUUID, + UUID: ptr.To("test-uuid"), }, }, }, }, }, - nclient: &mocknclient{}, - expectedChecks: 1, - expectedControlPlaneCheckFieldIncluded: false, + nclient: &mocknclient{ + getImageByIdFunc: func(uuid *string) (*vmmv4.GetImageApiResponse, error) { + assert.Equal(t, "test-uuid", *uuid) + resp := &vmmv4.GetImageApiResponse{} + err := resp.SetData(vmmv4.Image{ + ObjectType_: ptr.To("vmm.v4.content.Image"), + ExtId: ptr.To("test-uuid"), + }) + require.NoError(t, err) + return resp, nil + }, + }, + expectedChecks: 1, + expectedControlPlaneCheckFieldIncluded: false, expectedWorkerNodeCheckFieldPatternExists: true, }, { @@ -531,7 +544,7 @@ func TestInitVMImageChecks(t *testing.T) { MachineDetails: carenv1.NutanixMachineDetails{ Image: &capxv1.NutanixResourceIdentifier{ Type: capxv1.NutanixIdentifierUUID, - UUID: ptr.To("cp-uuid"), + UUID: ptr.To("test-uuid"), }, }, }, @@ -542,8 +555,8 @@ func TestInitVMImageChecks(t *testing.T) { Nutanix: &carenv1.NutanixNodeSpec{ MachineDetails: carenv1.NutanixMachineDetails{ Image: &capxv1.NutanixResourceIdentifier{ - Type: capxv1.NutanixIdentifierName, - Name: ptr.To("worker1-image"), + Type: capxv1.NutanixIdentifierUUID, + UUID: ptr.To("test-uuid"), }, }, }, @@ -552,16 +565,27 @@ func TestInitVMImageChecks(t *testing.T) { Nutanix: &carenv1.NutanixNodeSpec{ MachineDetails: carenv1.NutanixMachineDetails{ Image: &capxv1.NutanixResourceIdentifier{ - Type: capxv1.NutanixIdentifierName, - Name: ptr.To("worker2-image"), + Type: capxv1.NutanixIdentifierUUID, + UUID: ptr.To("test-uuid"), }, }, }, }, }, - nclient: &mocknclient{}, - expectedChecks: 3, // 1 control plane + 2 workers - expectedControlPlaneCheckFieldIncluded: true, + nclient: &mocknclient{ + getImageByIdFunc: func(uuid *string) (*vmmv4.GetImageApiResponse, error) { + assert.Equal(t, "test-uuid", *uuid) + resp := &vmmv4.GetImageApiResponse{} + err := resp.SetData(vmmv4.Image{ + ObjectType_: ptr.To("vmm.v4.content.Image"), + ExtId: ptr.To("test-uuid"), + }) + require.NoError(t, err) + return resp, nil + }, + }, + expectedChecks: 3, // 1 control plane + 2 workers + expectedControlPlaneCheckFieldIncluded: true, expectedWorkerNodeCheckFieldPatternExists: true, }, { @@ -575,16 +599,27 @@ func TestInitVMImageChecks(t *testing.T) { Nutanix: &carenv1.NutanixNodeSpec{ MachineDetails: carenv1.NutanixMachineDetails{ Image: &capxv1.NutanixResourceIdentifier{ - Type: capxv1.NutanixIdentifierName, - Name: ptr.To("worker2-image"), + Type: capxv1.NutanixIdentifierUUID, + UUID: ptr.To("test-uuid"), }, }, }, }, }, - nclient: &mocknclient{}, - expectedChecks: 1, // only worker-2 - expectedControlPlaneCheckFieldIncluded: false, + nclient: &mocknclient{ + getImageByIdFunc: func(uuid *string) (*vmmv4.GetImageApiResponse, error) { + assert.Equal(t, "test-uuid", *uuid) + resp := &vmmv4.GetImageApiResponse{} + err := resp.SetData(vmmv4.Image{ + ObjectType_: ptr.To("vmm.v4.content.Image"), + ExtId: ptr.To("test-uuid"), + }) + require.NoError(t, err) + return resp, nil + }, + }, + expectedChecks: 1, // only worker-2 + expectedControlPlaneCheckFieldIncluded: false, expectedWorkerNodeCheckFieldPatternExists: true, }, { @@ -622,51 +657,16 @@ func TestInitVMImageChecks(t *testing.T) { nclient: tc.nclient, } - // Trap the vmImageCheck calls to verify field paths - var capturedFields []string - checker.vmImageCheckFunc = func( - n *nutanixChecker, - machineDetails *carenv1.NutanixMachineDetails, - field string, - ) preflight.Check { - capturedFields = append(capturedFields, field) - return func(ctx context.Context) preflight.CheckResult { - // Simulate a successful check - return preflight.CheckResult{ - Name: "NutanixVMImage", - Allowed: true, - } - } - } - // Call the method under test checker.initVMImageChecksFunc = initVMImageChecks checks := checker.initVMImageChecksFunc(checker) // Verify number of checks assert.Len(t, checks, tc.expectedChecks) - assert.Len(t, capturedFields, tc.expectedChecks) - - // Verify field names in checks - if tc.expectedControlPlaneCheckFieldIncluded { - assert.Contains( - t, - capturedFields, - "cluster.spec.topology[.name=clusterConfig].value.controlPlane.nutanix.machineDetails", - ) - } - if tc.expectedWorkerNodeCheckFieldPatternExists { - foundWorkerFieldPattern := false - for _, field := range capturedFields { - if field != "cluster.spec.topology[.name=clusterConfig].value.controlPlane.nutanix.machineDetails" { - // This is not the control plane field, so it must be a worker node field - assert.Contains(t, field, "cluster.spec.topology.workers.machineDeployments[.name=") - assert.Contains(t, field, "].variables[.name=workerConfig].value.nutanix.machineDetails") - foundWorkerFieldPattern = true - } - } - assert.True(t, foundWorkerFieldPattern, "Worker node field pattern not found in any checks") + results := make([]preflight.CheckResult, len(checks)) + for i, check := range checks { + results[i] = check.Run(context.Background()) } }) } diff --git a/pkg/webhook/preflight/nutanix/specs.go b/pkg/webhook/preflight/nutanix/specs.go index 12b211077..3568df138 100644 --- a/pkg/webhook/preflight/nutanix/specs.go +++ b/pkg/webhook/preflight/nutanix/specs.go @@ -12,14 +12,27 @@ import ( "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight" ) +type configurationCheck struct { + result preflight.CheckResult +} + +func (c *configurationCheck) Name() string { + return "NutanixConfiguration" +} + +func (c *configurationCheck) Run(_ context.Context) preflight.CheckResult { + return c.result +} + func initNutanixConfiguration( n *nutanixChecker, ) preflight.Check { n.log.V(5).Info("Initializing Nutanix configuration check") - result := preflight.CheckResult{ - Name: "NutanixConfiguration", - Allowed: true, + configurationCheck := &configurationCheck{ + result: preflight.CheckResult{ + Allowed: true, + }, } nutanixClusterConfigSpec := &carenv1.NutanixClusterConfigSpec{} @@ -32,9 +45,9 @@ func initNutanixConfiguration( ) if err != nil { // Should not happen if the cluster passed CEL validation rules. - result.Allowed = false - result.Error = true - result.Causes = append(result.Causes, + configurationCheck.result.Allowed = false + configurationCheck.result.Error = true + configurationCheck.result.Causes = append(configurationCheck.result.Causes, preflight.Cause{ Message: fmt.Sprintf("Failed to unmarshal cluster variable %s: %s", carenv1.ClusterConfigVariableName, @@ -64,9 +77,9 @@ func initNutanixConfiguration( ) if err != nil { // Should not happen if the cluster passed CEL validation rules. - result.Allowed = false - result.Error = true - result.Causes = append(result.Causes, + configurationCheck.result.Allowed = false + configurationCheck.result.Error = true + configurationCheck.result.Causes = append(configurationCheck.result.Causes, preflight.Cause{ Message: fmt.Sprintf("Failed to unmarshal topology machineDeployment variable %s: %s", carenv1.WorkerConfigVariableName, @@ -91,7 +104,5 @@ func initNutanixConfiguration( n.nutanixWorkerNodeConfigSpecByMachineDeploymentName = nutanixWorkerNodeConfigSpecByMachineDeploymentName } - return func(ctx context.Context) preflight.CheckResult { - return result - } + return configurationCheck } diff --git a/pkg/webhook/preflight/nutanix/specs_test.go b/pkg/webhook/preflight/nutanix/specs_test.go index ca669e991..6f37df922 100644 --- a/pkg/webhook/preflight/nutanix/specs_test.go +++ b/pkg/webhook/preflight/nutanix/specs_test.go @@ -42,7 +42,6 @@ func TestNutanixChecker_initNutanixConfiguration(t *testing.T) { }, }, expectedResult: preflight.CheckResult{ - Name: "NutanixConfiguration", Allowed: true, }, expectedNutanixClusterConfigSpec: true, @@ -68,7 +67,6 @@ func TestNutanixChecker_initNutanixConfiguration(t *testing.T) { }, }, expectedResult: preflight.CheckResult{ - Name: "NutanixConfiguration", Allowed: true, }, expectedNutanixClusterConfigSpec: true, @@ -111,7 +109,6 @@ func TestNutanixChecker_initNutanixConfiguration(t *testing.T) { }, }, expectedResult: preflight.CheckResult{ - Name: "NutanixConfiguration", Allowed: true, }, expectedNutanixClusterConfigSpec: false, @@ -135,7 +132,6 @@ func TestNutanixChecker_initNutanixConfiguration(t *testing.T) { }, }, expectedResult: preflight.CheckResult{ - Name: "NutanixConfiguration", Allowed: false, Error: true, Causes: []preflight.Cause{ @@ -184,7 +180,6 @@ func TestNutanixChecker_initNutanixConfiguration(t *testing.T) { }, }, expectedResult: preflight.CheckResult{ - Name: "NutanixConfiguration", Allowed: false, Error: true, Causes: []preflight.Cause{ @@ -217,7 +212,6 @@ func TestNutanixChecker_initNutanixConfiguration(t *testing.T) { }, }, expectedResult: preflight.CheckResult{ - Name: "NutanixConfiguration", Allowed: true, }, expectedNutanixClusterConfigSpec: false, @@ -275,7 +269,6 @@ func TestNutanixChecker_initNutanixConfiguration(t *testing.T) { }, }, expectedResult: preflight.CheckResult{ - Name: "NutanixConfiguration", Allowed: true, }, expectedNutanixClusterConfigSpec: false, @@ -316,7 +309,6 @@ func TestNutanixChecker_initNutanixConfiguration(t *testing.T) { }, }, expectedResult: preflight.CheckResult{ - Name: "NutanixConfiguration", Allowed: true, }, expectedNutanixClusterConfigSpec: false, @@ -347,7 +339,6 @@ func TestNutanixChecker_initNutanixConfiguration(t *testing.T) { }, }, expectedResult: preflight.CheckResult{ - Name: "NutanixConfiguration", Allowed: true, }, expectedNutanixClusterConfigSpec: false, @@ -365,8 +356,8 @@ func TestNutanixChecker_initNutanixConfiguration(t *testing.T) { cluster: tt.cluster, } - checkFunc := initNutanixConfiguration(n) - result := checkFunc(context.Background()) + check := initNutanixConfiguration(n) + result := check.Run(context.Background()) assert.Equal(t, tt.expectedResult, result) From 91a8748d00e453709ce4c43277728f08a63ebf73 Mon Sep 17 00:00:00 2001 From: Daniel Lipovetsky Date: Fri, 13 Jun 2025 12:36:01 -0700 Subject: [PATCH 11/12] fixup! feat: Nutanix VM image preflight check Refactor to simplify, and remove need to checker factory. --- cmd/main.go | 2 +- pkg/webhook/preflight/nutanix/checker.go | 62 ++++----- pkg/webhook/preflight/nutanix/checker_test.go | 29 +++-- pkg/webhook/preflight/nutanix/credentials.go | 21 +-- .../preflight/nutanix/credentials_test.go | 121 +++++++++--------- pkg/webhook/preflight/nutanix/image.go | 22 ++-- pkg/webhook/preflight/nutanix/image_test.go | 21 +-- pkg/webhook/preflight/nutanix/specs.go | 16 +-- pkg/webhook/preflight/nutanix/specs_test.go | 16 +-- 9 files changed, 156 insertions(+), 154 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index d498bb023..c35bbb1ed 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -225,7 +225,7 @@ func main() { Handler: preflight.New(mgr.GetClient(), admission.NewDecoder(mgr.GetScheme()), []preflight.Checker{ // Add your preflight checkers here. - preflightnutanix.New, + preflightnutanix.Checker, }..., ), }) diff --git a/pkg/webhook/preflight/nutanix/checker.go b/pkg/webhook/preflight/nutanix/checker.go index bcae1fe5e..dc61ec445 100644 --- a/pkg/webhook/preflight/nutanix/checker.go +++ b/pkg/webhook/preflight/nutanix/checker.go @@ -17,58 +17,58 @@ import ( "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight" ) -func New(kclient ctrlclient.Client, cluster *clusterv1.Cluster) preflight.Checker { - return &nutanixChecker{ - kclient: kclient, - cluster: cluster, - - nclientFactory: newClient, - - initNutanixConfigurationFunc: initNutanixConfiguration, - initCredentialsCheckFunc: initCredentialsCheck, - initVMImageChecksFunc: initVMImageChecks, - } +var Checker = &nutanixChecker{ + configurationCheckFactory: newConfigurationCheck, + credentialsCheckFactory: newCredentialsCheck, + vmImageChecksFactory: newVMImageChecks, } type nutanixChecker struct { - kclient ctrlclient.Client - cluster *clusterv1.Cluster - - nutanixClusterConfigSpec *carenv1.NutanixClusterConfigSpec - nutanixWorkerNodeConfigSpecByMachineDeploymentName map[string]*carenv1.NutanixWorkerNodeConfigSpec - - nclient client - nclientFactory func(prismgoclient.Credentials) (client, error) - - initNutanixConfigurationFunc func( - n *nutanixChecker, + configurationCheckFactory func( + cd *checkDependencies, ) preflight.Check - initCredentialsCheckFunc func( + credentialsCheckFactory func( ctx context.Context, - n *nutanixChecker, + nclientFactory func(prismgoclient.Credentials) (client, error), + cd *checkDependencies, ) preflight.Check - initVMImageChecksFunc func( - n *nutanixChecker, + vmImageChecksFactory func( + cd *checkDependencies, ) []preflight.Check +} + +type checkDependencies struct { + kclient ctrlclient.Client + cluster *clusterv1.Cluster - log logr.Logger + nutanixClusterConfigSpec *carenv1.NutanixClusterConfigSpec + nutanixWorkerNodeConfigSpecByMachineDeploymentName map[string]*carenv1.NutanixWorkerNodeConfigSpec + + nclient client + log logr.Logger } func (n *nutanixChecker) Init( ctx context.Context, + kclient ctrlclient.Client, + cluster *clusterv1.Cluster, ) []preflight.Check { - n.log = ctrl.LoggerFrom(ctx).WithName("preflight/nutanix") + cd := &checkDependencies{ + kclient: kclient, + cluster: cluster, + log: ctrl.LoggerFrom(ctx).WithName("preflight/nutanix"), + } checks := []preflight.Check{ // The configuration check must run first, because it initializes data used by all other checks, // and the credentials check second, because it initializes the Nutanix clients used by other checks. - n.initNutanixConfigurationFunc(n), - n.initCredentialsCheckFunc(ctx, n), + n.configurationCheckFactory(cd), + n.credentialsCheckFactory(ctx, newClient, cd), } - checks = append(checks, n.initVMImageChecksFunc(n)...) + checks = append(checks, n.vmImageChecksFactory(cd)...) // Add more checks here as needed. diff --git a/pkg/webhook/preflight/nutanix/checker_test.go b/pkg/webhook/preflight/nutanix/checker_test.go index 8c6579d76..0e50a6d3a 100644 --- a/pkg/webhook/preflight/nutanix/checker_test.go +++ b/pkg/webhook/preflight/nutanix/checker_test.go @@ -9,8 +9,11 @@ import ( "testing" "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + prismgoclient "github.com/nutanix-cloud-native/prism-go-client" + carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight" ) @@ -98,18 +101,14 @@ func TestNutanixChecker_Init(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { // Create the checker - checker := &nutanixChecker{ - cluster: &clusterv1.Cluster{}, - nutanixClusterConfigSpec: tt.nutanixConfig, - nutanixWorkerNodeConfigSpecByMachineDeploymentName: tt.workerNodeConfigs, - } + checker := &nutanixChecker{} // Mock the sub-check functions to track their calls configCheckCalled := false credsCheckCalled := false vmImageCheckCount := 0 - checker.initNutanixConfigurationFunc = func(n *nutanixChecker) preflight.Check { + checker.configurationCheckFactory = func(cd *checkDependencies) preflight.Check { configCheckCalled = true return &mockCheck{ name: tt.expectedFirstCheckName, @@ -117,7 +116,11 @@ func TestNutanixChecker_Init(t *testing.T) { } } - checker.initCredentialsCheckFunc = func(ctx context.Context, n *nutanixChecker) preflight.Check { + checker.credentialsCheckFactory = func( + ctx context.Context, + nclientFactory func(prismgoclient.Credentials) (client, error), + cd *checkDependencies, + ) preflight.Check { credsCheckCalled = true return &mockCheck{ name: tt.expectedSecondCheckName, @@ -125,7 +128,7 @@ func TestNutanixChecker_Init(t *testing.T) { } } - checker.initVMImageChecksFunc = func(n *nutanixChecker) []preflight.Check { + checker.vmImageChecksFactory = func(cd *checkDependencies) []preflight.Check { checks := []preflight.Check{} for i := 0; i < tt.vmImageCheckCount; i++ { vmImageCheckCount++ @@ -143,10 +146,12 @@ func TestNutanixChecker_Init(t *testing.T) { // Call Init ctx := context.Background() - checks := checker.Init(ctx) - - // Verify the logger was set - assert.NotNil(t, checker.log) + checks := checker.Init(ctx, nil, &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + }) // Verify correct number of checks assert.Len(t, checks, tt.expectedCheckCount) diff --git a/pkg/webhook/preflight/nutanix/credentials.go b/pkg/webhook/preflight/nutanix/credentials.go index 30c2d88b0..e6530a612 100644 --- a/pkg/webhook/preflight/nutanix/credentials.go +++ b/pkg/webhook/preflight/nutanix/credentials.go @@ -30,11 +30,12 @@ func (c *credentialsCheck) Run(_ context.Context) preflight.CheckResult { return c.result } -func initCredentialsCheck( +func newCredentialsCheck( ctx context.Context, - n *nutanixChecker, + nclientFactory func(prismgoclient.Credentials) (client, error), + cd *checkDependencies, ) preflight.Check { - n.log.V(5).Info("Initializing Nutanix credentials check") + cd.log.V(5).Info("Initializing Nutanix credentials check") credentialsCheck := &credentialsCheck{ result: preflight.CheckResult{ @@ -42,14 +43,14 @@ func initCredentialsCheck( }, } - if n.nutanixClusterConfigSpec == nil && len(n.nutanixWorkerNodeConfigSpecByMachineDeploymentName) == 0 { + if cd.nutanixClusterConfigSpec == nil && len(cd.nutanixWorkerNodeConfigSpecByMachineDeploymentName) == 0 { // If there is no Nutanix configuration at all, the credentials check is not needed. return credentialsCheck } // There is some Nutanix configuration, so the credentials check is needed. // However, the credentials configuration is missing, so we cannot perform the check. - if n.nutanixClusterConfigSpec == nil || n.nutanixClusterConfigSpec.Nutanix == nil { + if cd.nutanixClusterConfigSpec == nil || cd.nutanixClusterConfigSpec.Nutanix == nil { credentialsCheck.result.Allowed = false credentialsCheck.result.Error = true credentialsCheck.result.Causes = append(credentialsCheck.result.Causes, @@ -62,7 +63,7 @@ func initCredentialsCheck( } // Get the credentials data in order to initialize the credentials and clients. - prismCentralEndpointSpec := n.nutanixClusterConfigSpec.Nutanix.PrismCentralEndpoint + prismCentralEndpointSpec := cd.nutanixClusterConfigSpec.Nutanix.PrismCentralEndpoint host, port, err := prismCentralEndpointSpec.ParseURL() if err != nil { @@ -79,10 +80,10 @@ func initCredentialsCheck( } credentialsSecret := &corev1.Secret{} - err = n.kclient.Get( + err = cd.kclient.Get( ctx, types.NamespacedName{ - Namespace: n.cluster.Namespace, + Namespace: cd.cluster.Namespace, Name: prismCentralEndpointSpec.Credentials.SecretRef.Name, }, credentialsSecret, @@ -154,7 +155,7 @@ func initCredentialsCheck( } // Initialize the Nutanix client. - nclient, err := n.nclientFactory(credentials) + nclient, err := nclientFactory(credentials) if err != nil { credentialsCheck.result.Allowed = false credentialsCheck.result.Error = true @@ -183,7 +184,7 @@ func initCredentialsCheck( } // We initialized both clients, and verified the credentials using the v3 client. - n.nclient = nclient + cd.nclient = nclient return credentialsCheck } diff --git a/pkg/webhook/preflight/nutanix/credentials_test.go b/pkg/webhook/preflight/nutanix/credentials_test.go index ac5239209..41d909fb8 100644 --- a/pkg/webhook/preflight/nutanix/credentials_test.go +++ b/pkg/webhook/preflight/nutanix/credentials_test.go @@ -18,30 +18,38 @@ import ( carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" ) -func TestInitCredentialsCheck_Success(t *testing.T) { - nc := validNutanixChecker() - check := nc.initCredentialsCheckFunc(context.Background(), nc) +func TestNewCredentialsCheck_Success(t *testing.T) { + cd := validCheckDependencies() + nclientFactory := func(_ prismgoclient.Credentials) (client, error) { + return &mocknclient{}, nil + } + check := newCredentialsCheck(context.Background(), nclientFactory, cd) result := check.Run(context.Background()) assert.True(t, result.Allowed) assert.False(t, result.Error) assert.Empty(t, result.Causes) } -func TestInitCredentialsCheck_NoNutanixConfig(t *testing.T) { - nc := validNutanixChecker() - nc.nutanixClusterConfigSpec = nil - nc.nutanixWorkerNodeConfigSpecByMachineDeploymentName = map[string]*carenv1.NutanixWorkerNodeConfigSpec{} - check := nc.initCredentialsCheckFunc(context.Background(), nc) +func TestNewCredentialsCheck_NoNutanixConfig(t *testing.T) { + cd := validCheckDependencies() + cd.nutanixClusterConfigSpec = nil + nclientFactory := func(_ prismgoclient.Credentials) (client, error) { + return &mocknclient{}, nil + } + check := newCredentialsCheck(context.Background(), nclientFactory, cd) result := check.Run(context.Background()) assert.True(t, result.Allowed) assert.False(t, result.Error) assert.Empty(t, result.Causes) } -func TestInitCredentialsCheck_MissingNutanixField(t *testing.T) { - nc := validNutanixChecker() - nc.nutanixClusterConfigSpec.Nutanix = nil - check := nc.initCredentialsCheckFunc(context.Background(), nc) +func TestNewCredentialsCheck_MissingNutanixField(t *testing.T) { + cd := validCheckDependencies() + cd.nutanixClusterConfigSpec.Nutanix = nil + nclientFactory := func(_ prismgoclient.Credentials) (client, error) { + return &mocknclient{}, nil + } + check := newCredentialsCheck(context.Background(), nclientFactory, cd) result := check.Run(context.Background()) assert.False(t, result.Allowed) assert.True(t, result.Error) @@ -49,27 +57,33 @@ func TestInitCredentialsCheck_MissingNutanixField(t *testing.T) { assert.Contains(t, result.Causes[0].Message, "Nutanix cluster configuration is not defined") } -func TestInitCredentialsCheck_InvalidURL(t *testing.T) { - nc := validNutanixChecker() - nc.nutanixClusterConfigSpec.Nutanix.PrismCentralEndpoint.URL = "not-a-url" - check := nc.initCredentialsCheckFunc(context.Background(), nc) +func TestNewCredentialsCheck_InvalidURL(t *testing.T) { + cd := validCheckDependencies() + cd.nutanixClusterConfigSpec.Nutanix.PrismCentralEndpoint.URL = "not-a-url" + nclientFactory := func(_ prismgoclient.Credentials) (client, error) { + return &mocknclient{}, nil + } + check := newCredentialsCheck(context.Background(), nclientFactory, cd) result := check.Run(context.Background()) assert.False(t, result.Allowed) assert.True(t, result.Error) assert.Contains(t, result.Causes[0].Message, "failed to parse Prism Central endpoint URL") } -func TestInitCredentialsCheck_SecretNotFound(t *testing.T) { - nc := validNutanixChecker() - nc.kclient = fake.NewClientBuilder().Build() // no secret - check := nc.initCredentialsCheckFunc(context.Background(), nc) +func TestNewCredentialsCheck_SecretNotFound(t *testing.T) { + cd := validCheckDependencies() + cd.kclient = fake.NewClientBuilder().Build() // no secret + nclientFactory := func(_ prismgoclient.Credentials) (client, error) { + return &mocknclient{}, nil + } + check := newCredentialsCheck(context.Background(), nclientFactory, cd) result := check.Run(context.Background()) assert.False(t, result.Allowed) assert.True(t, result.Error) assert.Contains(t, result.Causes[0].Message, "failed to get Prism Central credentials Secret") } -func TestInitCredentialsCheck_SecretEmpty(t *testing.T) { +func TestNewCredentialsCheck_SecretEmpty(t *testing.T) { secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "ntnx-creds", @@ -77,17 +91,19 @@ func TestInitCredentialsCheck_SecretEmpty(t *testing.T) { }, Data: map[string][]byte{}, } - kclient := fake.NewClientBuilder().WithObjects(secret).Build() - nc := validNutanixChecker() - nc.kclient = kclient - check := nc.initCredentialsCheckFunc(context.Background(), nc) + cd := validCheckDependencies() + cd.kclient = fake.NewClientBuilder().WithObjects(secret).Build() + nclientFactory := func(_ prismgoclient.Credentials) (client, error) { + return &mocknclient{}, nil + } + check := newCredentialsCheck(context.Background(), nclientFactory, cd) result := check.Run(context.Background()) assert.False(t, result.Allowed) assert.True(t, result.Error) assert.Contains(t, result.Causes[0].Message, "credentials Secret 'ntnx-creds' is empty") } -func TestInitCredentialsCheck_SecretMissingKey(t *testing.T) { +func TestNewCredentialsCheck_SecretMissingKey(t *testing.T) { secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "ntnx-creds", @@ -97,17 +113,16 @@ func TestInitCredentialsCheck_SecretMissingKey(t *testing.T) { "not-credentials": []byte("foo"), }, } - kclient := fake.NewClientBuilder().WithObjects(secret).Build() - nc := validNutanixChecker() - nc.kclient = kclient - check := nc.initCredentialsCheckFunc(context.Background(), nc) + cd := validCheckDependencies() + cd.kclient = fake.NewClientBuilder().WithObjects(secret).Build() + check := newCredentialsCheck(context.Background(), nil, cd) result := check.Run(context.Background()) assert.False(t, result.Allowed) assert.True(t, result.Error) assert.Contains(t, result.Causes[0].Message, "does not contain key 'credentials'") } -func TestInitCredentialsCheck_InvalidCredentialsFormat(t *testing.T) { +func TestNewCredentialsCheck_InvalidCredentialsFormat(t *testing.T) { secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "ntnx-creds", @@ -117,45 +132,45 @@ func TestInitCredentialsCheck_InvalidCredentialsFormat(t *testing.T) { "credentials": []byte("not-a-valid-format"), }, } - kclient := fake.NewClientBuilder().WithObjects(secret).Build() - nc := validNutanixChecker() - nc.kclient = kclient - check := nc.initCredentialsCheckFunc(context.Background(), nc) + cd := validCheckDependencies() + cd.kclient = fake.NewClientBuilder().WithObjects(secret).Build() + nclientFactory := func(_ prismgoclient.Credentials) (client, error) { + return &mocknclient{}, nil + } + check := newCredentialsCheck(context.Background(), nclientFactory, cd) result := check.Run(context.Background()) assert.False(t, result.Allowed) assert.True(t, result.Error) assert.Contains(t, result.Causes[0].Message, "failed to parse Prism Central credentials") } -func TestInitCredentialsCheck_FailedToCreateClient(t *testing.T) { +func TestNewCredentialsCheck_FailedToCreateClient(t *testing.T) { // Simulate a failure in creating the v4 client - nc := validNutanixChecker() - nc.nclientFactory = func(_ prismgoclient.Credentials) (client, error) { + nclientFactory := func(_ prismgoclient.Credentials) (client, error) { return nil, assert.AnError } - - check := nc.initCredentialsCheckFunc(context.Background(), nc) + cd := validCheckDependencies() + check := newCredentialsCheck(context.Background(), nclientFactory, cd) result := check.Run(context.Background()) assert.False(t, result.Allowed) assert.True(t, result.Error) assert.Contains(t, result.Causes[0].Message, "Failed to initialize Nutanix client") } -func TestInitCredentialsCheck_FailedToGetCurrentLoggedInUser(t *testing.T) { +func TestNewCredentialsCheck_FailedToGetCurrentLoggedInUser(t *testing.T) { // Simulate a failure in getting the current logged-in user - nc := validNutanixChecker() - nc.nclientFactory = func(_ prismgoclient.Credentials) (client, error) { + nclientFactory := func(_ prismgoclient.Credentials) (client, error) { return &mocknclient{err: assert.AnError}, nil } - - check := nc.initCredentialsCheckFunc(context.Background(), nc) + cd := validCheckDependencies() + check := newCredentialsCheck(context.Background(), nclientFactory, cd) result := check.Run(context.Background()) assert.False(t, result.Allowed) assert.True(t, result.Error) assert.Contains(t, result.Causes[0].Message, "Failed to validate credentials using the v3 API client.") } -func validNutanixChecker() *nutanixChecker { +func validCheckDependencies() *checkDependencies { secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "ntnx-creds", @@ -175,23 +190,15 @@ func validNutanixChecker() *nutanixChecker { ]`), }, } - kclient := fake.NewClientBuilder().WithObjects(secret).Build() - return &nutanixChecker{ - kclient: kclient, + + return &checkDependencies{ + kclient: fake.NewClientBuilder().WithObjects(secret).Build(), cluster: &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", Namespace: "default", }, }, - - nclientFactory: func(_ prismgoclient.Credentials) (client, error) { - return &mocknclient{}, nil - }, - - initNutanixConfigurationFunc: initNutanixConfiguration, - initCredentialsCheckFunc: initCredentialsCheck, - nutanixClusterConfigSpec: &carenv1.NutanixClusterConfigSpec{ Nutanix: &carenv1.NutanixSpec{ PrismCentralEndpoint: carenv1.NutanixPrismCentralEndpointSpec{ diff --git a/pkg/webhook/preflight/nutanix/image.go b/pkg/webhook/preflight/nutanix/image.go index 9224d4a57..a8298139c 100644 --- a/pkg/webhook/preflight/nutanix/image.go +++ b/pkg/webhook/preflight/nutanix/image.go @@ -17,7 +17,7 @@ import ( type imageCheck struct { machineDetails *carenv1.NutanixMachineDetails field string - n *nutanixChecker + nclient client } func (c *imageCheck) Name() string { @@ -39,7 +39,7 @@ func (c *imageCheck) Run(ctx context.Context) preflight.CheckResult { } if c.machineDetails.Image != nil { - images, err := getVMImages(c.n.nclient, c.machineDetails.Image) + images, err := getVMImages(c.nclient, c.machineDetails.Image) if err != nil { result.Allowed = false result.Error = true @@ -68,35 +68,35 @@ func (c *imageCheck) Run(ctx context.Context) preflight.CheckResult { return result } -func initVMImageChecks( - n *nutanixChecker, +func newVMImageChecks( + cd *checkDependencies, ) []preflight.Check { checks := []preflight.Check{} - if n.nclient == nil { + if cd.nclient == nil { return checks } - if n.nutanixClusterConfigSpec != nil && n.nutanixClusterConfigSpec.ControlPlane != nil && - n.nutanixClusterConfigSpec.ControlPlane.Nutanix != nil { + if cd.nutanixClusterConfigSpec != nil && cd.nutanixClusterConfigSpec.ControlPlane != nil && + cd.nutanixClusterConfigSpec.ControlPlane.Nutanix != nil { checks = append(checks, &imageCheck{ - machineDetails: &n.nutanixClusterConfigSpec.ControlPlane.Nutanix.MachineDetails, + machineDetails: &cd.nutanixClusterConfigSpec.ControlPlane.Nutanix.MachineDetails, field: "cluster.spec.topology.variables[.name=clusterConfig]" + ".value.nutanix.controlPlane.machineDetails", - n: n, + nclient: cd.nclient, }, ) } - for mdName, nutanixWorkerNodeConfigSpec := range n.nutanixWorkerNodeConfigSpecByMachineDeploymentName { + for mdName, nutanixWorkerNodeConfigSpec := range cd.nutanixWorkerNodeConfigSpecByMachineDeploymentName { if nutanixWorkerNodeConfigSpec.Nutanix != nil { checks = append(checks, &imageCheck{ machineDetails: &nutanixWorkerNodeConfigSpec.Nutanix.MachineDetails, field: fmt.Sprintf("cluster.spec.topology.workers.machineDeployments[.name=%s]"+ ".variables[.name=workerConfig].value.nutanix.machineDetails", mdName), - n: n, + nclient: cd.nclient, }, ) } diff --git a/pkg/webhook/preflight/nutanix/image_test.go b/pkg/webhook/preflight/nutanix/image_test.go index 09b9f2ae4..e8083873d 100644 --- a/pkg/webhook/preflight/nutanix/image_test.go +++ b/pkg/webhook/preflight/nutanix/image_test.go @@ -244,18 +244,11 @@ func TestVMImageCheck(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - logger := testr.New(t) - - checker := &nutanixChecker{ - log: logger, - nclient: tc.nclient, - } - // Create the check check := &imageCheck{ machineDetails: tc.machineDetails, field: "test-field", - n: checker, + nclient: tc.nclient, } // Execute the check @@ -446,7 +439,7 @@ func TestGetVMImages(t *testing.T) { } } -func TestInitVMImageChecks(t *testing.T) { +func TestNewVMImageChecks(t *testing.T) { testCases := []struct { name string nutanixClusterConfigSpec *carenv1.NutanixClusterConfigSpec @@ -649,17 +642,15 @@ func TestInitVMImageChecks(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - logger := testr.New(t) - checker := &nutanixChecker{ - log: logger, - nutanixClusterConfigSpec: tc.nutanixClusterConfigSpec, + cd := &checkDependencies{ + nutanixClusterConfigSpec: tc.nutanixClusterConfigSpec, nutanixWorkerNodeConfigSpecByMachineDeploymentName: tc.nutanixWorkerNodeConfigSpecByMDName, nclient: tc.nclient, + log: testr.New(t), } // Call the method under test - checker.initVMImageChecksFunc = initVMImageChecks - checks := checker.initVMImageChecksFunc(checker) + checks := newVMImageChecks(cd) // Verify number of checks assert.Len(t, checks, tc.expectedChecks) diff --git a/pkg/webhook/preflight/nutanix/specs.go b/pkg/webhook/preflight/nutanix/specs.go index 3568df138..ae1b98722 100644 --- a/pkg/webhook/preflight/nutanix/specs.go +++ b/pkg/webhook/preflight/nutanix/specs.go @@ -24,10 +24,10 @@ func (c *configurationCheck) Run(_ context.Context) preflight.CheckResult { return c.result } -func initNutanixConfiguration( - n *nutanixChecker, +func newConfigurationCheck( + cd *checkDependencies, ) preflight.Check { - n.log.V(5).Info("Initializing Nutanix configuration check") + cd.log.V(5).Info("Initializing Nutanix configuration check") configurationCheck := &configurationCheck{ result: preflight.CheckResult{ @@ -39,7 +39,7 @@ func initNutanixConfiguration( err := variables.UnmarshalClusterVariable( variables.GetClusterVariableByName( carenv1.ClusterConfigVariableName, - n.cluster.Spec.Topology.Variables, + cd.cluster.Spec.Topology.Variables, ), nutanixClusterConfigSpec, ) @@ -61,12 +61,12 @@ func initNutanixConfiguration( // Save the NutanixClusterConfigSpec only if it contains Nutanix configuration. if nutanixClusterConfigSpec.Nutanix != nil || (nutanixClusterConfigSpec.ControlPlane != nil && nutanixClusterConfigSpec.ControlPlane.Nutanix != nil) { - n.nutanixClusterConfigSpec = nutanixClusterConfigSpec + cd.nutanixClusterConfigSpec = nutanixClusterConfigSpec } nutanixWorkerNodeConfigSpecByMachineDeploymentName := make(map[string]*carenv1.NutanixWorkerNodeConfigSpec) - if n.cluster.Spec.Topology.Workers != nil { - for _, md := range n.cluster.Spec.Topology.Workers.MachineDeployments { + if cd.cluster.Spec.Topology.Workers != nil { + for _, md := range cd.cluster.Spec.Topology.Workers.MachineDeployments { if md.Variables == nil { continue } @@ -101,7 +101,7 @@ func initNutanixConfiguration( } // Save the NutanixWorkerNodeConfigSpecByMachineDeploymentName only if it contains at least one Nutanix configuration. if len(nutanixWorkerNodeConfigSpecByMachineDeploymentName) > 0 { - n.nutanixWorkerNodeConfigSpecByMachineDeploymentName = nutanixWorkerNodeConfigSpecByMachineDeploymentName + cd.nutanixWorkerNodeConfigSpecByMachineDeploymentName = nutanixWorkerNodeConfigSpecByMachineDeploymentName } return configurationCheck diff --git a/pkg/webhook/preflight/nutanix/specs_test.go b/pkg/webhook/preflight/nutanix/specs_test.go index 6f37df922..a5ddf2711 100644 --- a/pkg/webhook/preflight/nutanix/specs_test.go +++ b/pkg/webhook/preflight/nutanix/specs_test.go @@ -16,7 +16,7 @@ import ( "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight" ) -func TestNutanixChecker_initNutanixConfiguration(t *testing.T) { +func TestNewConfigurationCheck(t *testing.T) { tests := []struct { name string cluster *clusterv1.Cluster @@ -349,28 +349,26 @@ func TestNutanixChecker_initNutanixConfiguration(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - logger := testr.New(t) - - n := &nutanixChecker{ - log: logger, + cd := &checkDependencies{ cluster: tt.cluster, + log: testr.New(t), } - check := initNutanixConfiguration(n) + check := newConfigurationCheck(cd) result := check.Run(context.Background()) assert.Equal(t, tt.expectedResult, result) - hasNutanixClusterConfigSpec := n.nutanixClusterConfigSpec != nil + hasNutanixClusterConfigSpec := cd.nutanixClusterConfigSpec != nil assert.Equal(t, tt.expectedNutanixClusterConfigSpec, hasNutanixClusterConfigSpec) - hasWorkerNodeConfigSpecMap := n.nutanixWorkerNodeConfigSpecByMachineDeploymentName != nil + hasWorkerNodeConfigSpecMap := cd.nutanixWorkerNodeConfigSpecByMachineDeploymentName != nil assert.Equal(t, tt.expectedWorkerNodeConfigSpecMapNotEmpty, hasWorkerNodeConfigSpecMap) if hasWorkerNodeConfigSpecMap { assert.Len( t, - n.nutanixWorkerNodeConfigSpecByMachineDeploymentName, tt.expectedWorkerNodeConfigSpecMapEntryCount, + cd.nutanixWorkerNodeConfigSpecByMachineDeploymentName, tt.expectedWorkerNodeConfigSpecMapEntryCount, ) } }) From ed9b787e517ab6a468beebcc24b04a6c4a00bb99 Mon Sep 17 00:00:00 2001 From: Daniel Lipovetsky Date: Tue, 17 Jun 2025 09:08:38 -0700 Subject: [PATCH 12/12] fixup! feat: Nutanix VM image preflight check Address linter complaint --- pkg/webhook/preflight/nutanix/specs.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/webhook/preflight/nutanix/specs.go b/pkg/webhook/preflight/nutanix/specs.go index ae1b98722..cd802d810 100644 --- a/pkg/webhook/preflight/nutanix/specs.go +++ b/pkg/webhook/preflight/nutanix/specs.go @@ -66,7 +66,8 @@ func newConfigurationCheck( nutanixWorkerNodeConfigSpecByMachineDeploymentName := make(map[string]*carenv1.NutanixWorkerNodeConfigSpec) if cd.cluster.Spec.Topology.Workers != nil { - for _, md := range cd.cluster.Spec.Topology.Workers.MachineDeployments { + for i := range cd.cluster.Spec.Topology.Workers.MachineDeployments { + md := &cd.cluster.Spec.Topology.Workers.MachineDeployments[i] if md.Variables == nil { continue }