Adding HyperV provider #131
Conversation
WalkthroughThe changes add HyperV provider support to the system, including a new provider creation workflow with secret management, input validation of required options, and registration of hyperv as a valid provider type in the CLI validation framework. Changes
Sequence DiagramsequenceDiagram
participant User
participant CreateProvider as CreateProvider<br/>(hyperv)
participant Validate as Validate<br/>Options
participant SecretMgmt as Secret<br/>Management
participant K8sClient as Kubernetes<br/>Client
participant DynClient as Dynamic<br/>Client
User->>CreateProvider: Call CreateProvider()
CreateProvider->>Validate: Validate name, namespace,<br/>URL, credentials
alt Validation fails
Validate-->>CreateProvider: Error
CreateProvider-->>User: Return error
end
Validate-->>CreateProvider: OK
CreateProvider->>CreateProvider: Build Provider object<br/>(type: hyperv)
alt No existing secret provided
CreateProvider->>SecretMgmt: Create new secret
SecretMgmt->>K8sClient: Create Secret resource<br/>(username, password)
alt Secret creation fails
K8sClient-->>SecretMgmt: Error
SecretMgmt-->>CreateProvider: Error
CreateProvider-->>User: Return error
end
K8sClient-->>SecretMgmt: Secret created
SecretMgmt->>CreateProvider: Return secret reference
else Use existing secret
CreateProvider->>CreateProvider: Wire existing secret<br/>to Provider
end
CreateProvider->>DynClient: Create Provider resource
alt Provider creation fails
DynClient-->>CreateProvider: Error
alt Secret was created
CreateProvider->>SecretMgmt: Cleanup secret
SecretMgmt->>K8sClient: Delete secret
end
CreateProvider-->>User: Return error
end
DynClient-->>CreateProvider: Provider created
alt Secret was created
CreateProvider->>SecretMgmt: Set OwnerReference
SecretMgmt->>K8sClient: Patch secret with<br/>OwnerReference
K8sClient-->>SecretMgmt: OK
SecretMgmt-->>CreateProvider: OK
end
CreateProvider-->>User: Return Provider & Secret
Estimated Code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pkg/cmd/create/provider/hyperv/hyperv.go (1)
19-38: Consider whether InsecureSkipTLS and CACert options are applicable for HyperV.The validation only requires name, namespace, URL, username, and password. Other providers (like vsphere, generic) also support
InsecureSkipTLSandCACertoptions for TLS configuration. If HyperV connections can involve TLS (e.g., WinRM over HTTPS), consider whether these options should be supported.If HyperV exclusively uses SMB shares without TLS requirements, this is fine as-is.
pkg/cmd/create/provider/hyperv/secrets.go (1)
67-87: Using MergePatch replaces existing ownerReferences.
MergePatchTypewill replace the entireownerReferencesarray rather than appending to it. This is fine for freshly created secrets (which have no existing ownerReferences), but be aware of this behavior if this function is ever reused for secrets that might have pre-existing owner references.For future-proofing,
StrategicMergePatchTypewould append to the array, but since this code path only applies to newly created secrets, the current implementation is correct.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
pkg/cmd/create/provider/create.go(2 hunks)pkg/cmd/create/provider/hyperv/hyperv.go(1 hunks)pkg/cmd/create/provider/hyperv/secrets.go(1 hunks)pkg/util/flags/provider_type.go(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
pkg/cmd/create/provider/create.go (2)
pkg/cmd/create/provider/hyperv/hyperv.go (1)
CreateProvider(91-151)pkg/cmd/create/provider/generic/generic.go (1)
CreateProvider(146-204)
pkg/cmd/create/provider/hyperv/hyperv.go (3)
pkg/cmd/create/provider/providerutil/options.go (1)
ProviderOptions(4-29)pkg/util/client/client.go (2)
GetDynamicClient(108-120)SecretsGVR(86-90)pkg/cmd/create/provider/create.go (1)
Create(22-97)
pkg/cmd/create/provider/hyperv/secrets.go (1)
pkg/util/client/client.go (1)
GetKubernetesClientset(123-135)
🔇 Additional comments (5)
pkg/cmd/create/provider/create.go (1)
66-67: LGTM!The HyperV provider case is correctly integrated into the switch statement, following the same pattern as other providers like vsphere and ova.
pkg/util/flags/provider_type.go (1)
27-28: LGTM!The addition of "hyperv" to static types is consistent with how "ec2" is handled. The type is correctly added in all three locations: staticTypes validation, error message, and GetValidValues for auto-completion.
pkg/cmd/create/provider/hyperv/hyperv.go (2)
56-88: LGTM!The
createTypedProviderfunction correctly handles the typed-to-unstructured-to-typed conversion flow with proper error handling.
90-151: LGTM!The
CreateProviderfunction follows the established pattern from other providers with proper validation, secret handling, error cleanup, and ownership binding.pkg/cmd/create/provider/hyperv/secrets.go (1)
18-49: LGTM!The
createSecretfunction correctly creates an opaque Secret with proper labels and usesGenerateNamefor unique naming.
|
https://github.com/yaacov/kubectl-mtv/releases/tag/v0.1.58 - new release 🎉 |
2 participants
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.