Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: validation enforcing vnet guid is set for overlay cni clusters #675

Merged
merged 17 commits into from
Feb 21, 2025
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
1bbaa2d
feat: validation enforcing vnet guid is set for overlay cni clusters
Bryce-Soghigian Feb 5, 2025
63d177c
fix: deps
Bryce-Soghigian Feb 7, 2025
2f91352
test: adding test for the azurecni with overlay guid validation
Bryce-Soghigian Feb 7, 2025
d3e1796
test: adding validation to validate shape looks like a guid, leverage…
Bryce-Soghigian Feb 7, 2025
1325171
Merge branch 'main' into bsoghigian/removing-dep-after-rollout
Bryce-Soghigian Feb 7, 2025
6b4e911
ci: golangci-lint run
Bryce-Soghigian Feb 7, 2025
7099b64
deps: moving google uuid from an indirect dependency to a direct one
Bryce-Soghigian Feb 7, 2025
d5c99a4
refactor: updating options to use a proper guid
Bryce-Soghigian Feb 7, 2025
b39b4e3
test: network-plugin-mode falls back to overlay when unset which trig…
Bryce-Soghigian Feb 7, 2025
6600543
ci: golangci-lint run
Bryce-Soghigian Feb 7, 2025
8bdc099
test: testing happy path of properly configured vnet guid
Bryce-Soghigian Feb 7, 2025
2568c98
fix: validate all vnet guids that are passed in even if we don't use …
Bryce-Soghigian Feb 16, 2025
06c457a
Merge branch 'main' into bsoghigian/removing-dep-after-rollout
Bryce-Soghigian Feb 16, 2025
b6c68ec
Merge branch 'main' into bsoghigian/removing-dep-after-rollout
Bryce-Soghigian Feb 18, 2025
5e0847e
Merge branch 'main' into bsoghigian/removing-dep-after-rollout
tallaxes Feb 21, 2025
9eab563
Merge branch 'main' into bsoghigian/removing-dep-after-rollout
tallaxes Feb 21, 2025
593f2f3
test: resolve merge conflic
tallaxes Feb 21, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ require (
github.com/go-openapi/validate v0.24.0
github.com/go-playground/validator/v10 v10.25.0
github.com/google/go-cmp v0.6.0
github.com/google/uuid v1.6.0
github.com/imdario/mergo v0.3.16
github.com/jongio/azidext/go/azidext v0.5.0
github.com/mitchellh/hashstructure/v2 v2.0.2
Expand Down Expand Up @@ -106,7 +107,6 @@ require (
github.com/google/gnostic-models v0.6.8 // indirect
github.com/google/gofuzz v1.2.0 // indirect
github.com/google/pprof v0.0.0-20241210010833-40e02aabc2ad // indirect
github.com/google/uuid v1.6.0 // indirect
github.com/grpc-ecosystem/grpc-gateway/v2 v2.21.0 // indirect
github.com/hashicorp/golang-lru v1.0.2 // indirect
github.com/inconshreveable/mousetrap v1.1.0 // indirect
Expand Down
37 changes: 0 additions & 37 deletions pkg/operator/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,9 @@ import (
"sigs.k8s.io/karpenter/pkg/operator/injection"
karpenteroptions "sigs.k8s.io/karpenter/pkg/operator/options"

"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork"

"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
webhooksalt "github.com/Azure/karpenter-provider-azure/pkg/alt/karpenter-core/pkg/webhooks"
"github.com/Azure/karpenter-provider-azure/pkg/auth"
azurecache "github.com/Azure/karpenter-provider-azure/pkg/cache"
"github.com/Azure/karpenter-provider-azure/pkg/consts"

"github.com/Azure/karpenter-provider-azure/pkg/operator/options"
"github.com/Azure/karpenter-provider-azure/pkg/providers/imagefamily"
Expand All @@ -55,8 +51,6 @@ import (
"github.com/Azure/karpenter-provider-azure/pkg/providers/launchtemplate"
"github.com/Azure/karpenter-provider-azure/pkg/providers/loadbalancer"
"github.com/Azure/karpenter-provider-azure/pkg/providers/pricing"
"github.com/Azure/karpenter-provider-azure/pkg/utils"
armopts "github.com/Azure/karpenter-provider-azure/pkg/utils/opts"
"sigs.k8s.io/karpenter/pkg/operator"
)

Expand Down Expand Up @@ -89,12 +83,6 @@ func NewOperator(ctx context.Context, operator *operator.Operator) (context.Cont
azClient, err := instance.CreateAZClient(ctx, azConfig)
lo.Must0(err, "creating Azure client")

if options.FromContext(ctx).VnetGUID == "" && options.FromContext(ctx).NetworkPluginMode == consts.NetworkPluginModeOverlay {
vnetGUID, err := getVnetGUID(azConfig, options.FromContext(ctx).SubnetID)
lo.Must0(err, "getting VNET GUID")
options.FromContext(ctx).VnetGUID = vnetGUID
}

unavailableOfferingsCache := azurecache.NewUnavailableOfferings()
pricingProvider := pricing.NewProvider(
ctx,
Expand Down Expand Up @@ -235,28 +223,3 @@ func getCABundle(restConfig *rest.Config) (*string, error) {
}
return ptr.String(base64.StdEncoding.EncodeToString(transportConfig.TLS.CAData)), nil
}

func getVnetGUID(cfg *auth.Config, subnetID string) (string, error) {
creds, err := azidentity.NewDefaultAzureCredential(nil)
if err != nil {
return "", err
}
opts := armopts.DefaultArmOpts()
vnetClient, err := armnetwork.NewVirtualNetworksClient(cfg.SubscriptionID, creds, opts)
if err != nil {
return "", err
}

subnetParts, err := utils.GetVnetSubnetIDComponents(subnetID)
if err != nil {
return "", err
}
vnet, err := vnetClient.Get(context.Background(), subnetParts.ResourceGroupName, subnetParts.VNetName, nil)
if err != nil {
return "", err
}
if vnet.Properties == nil || vnet.Properties.ResourceGUID == nil {
return "", fmt.Errorf("vnet %s does not have a resource GUID", subnetParts.VNetName)
}
return *vnet.Properties.ResourceGUID, nil
}
16 changes: 16 additions & 0 deletions pkg/operator/options/options_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,15 @@ import (
"github.com/Azure/karpenter-provider-azure/pkg/consts"
"github.com/Azure/karpenter-provider-azure/pkg/utils"
"github.com/go-playground/validator/v10"
"github.com/google/uuid"
"go.uber.org/multierr"
)

func (o Options) Validate() error {
validate := validator.New()
return multierr.Combine(
o.validateRequiredFields(),
o.validateVNETGUID(),
o.validateEndpoint(),
o.validateNetworkingOptions(),
o.validateVMMemoryOverheadPercent(),
Expand All @@ -39,6 +41,16 @@ func (o Options) Validate() error {
)
}

func (o Options) validateVNETGUID() error {
if o.VnetGUID != "" && uuid.Validate(o.VnetGUID) != nil {
return fmt.Errorf("vnet-guid %s is malformed", o.VnetGUID)
}
if o.isAzureCNIWithOverlay() && o.VnetGUID == "" {
return fmt.Errorf("vnet-guid cannot be empty for AzureCNI clusters with networkPluginMode overlay")
}
return nil
}

func (o Options) validateNetworkingOptions() error {
if o.NetworkPlugin != consts.NetworkPluginAzure && o.NetworkPlugin != consts.NetworkPluginNone {
return fmt.Errorf("network-plugin %v is invalid. network-plugin must equal 'azure' or 'none'", o.NetworkPlugin)
Expand All @@ -56,6 +68,10 @@ func (o Options) validateNetworkingOptions() error {
return nil
}

func (o Options) isAzureCNIWithOverlay() bool {
return o.NetworkPlugin == consts.NetworkPluginAzure && o.NetworkPluginMode == consts.NetworkPluginModeOverlay
}

func (o Options) validateVnetSubnetID() error {
_, err := utils.GetVnetSubnetIDComponents(o.SubnetID)
if err != nil {
Expand Down
49 changes: 49 additions & 0 deletions pkg/operator/options/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ var _ = Describe("Options", func() {
"NODE_IDENTITIES",
"PROVISION_MODE",
"NODEBOOTSTRAPPING_SERVER_URL",
"VNET_GUID",
}

var fs *coreoptions.FlagSet
Expand Down Expand Up @@ -99,6 +100,7 @@ var _ = Describe("Options", func() {
os.Setenv("VNET_SUBNET_ID", "/subscriptions/12345678-1234-1234-1234-123456789012/resourceGroups/sillygeese/providers/Microsoft.Network/virtualNetworks/karpentervnet/subnets/karpentersub")
os.Setenv("PROVISION_MODE", "bootstrappingclient")
os.Setenv("NODEBOOTSTRAPPING_SERVER_URL", "https://nodebootstrapping-server-url")
os.Setenv("VNET_GUID", "a519e60a-cac0-40b2-b883-084477fe6f5c")
fs = &coreoptions.FlagSet{
FlagSet: flag.NewFlagSet("karpenter", flag.ContinueOnError),
}
Expand All @@ -118,10 +120,41 @@ var _ = Describe("Options", func() {
NodeIdentities: []string{"/subscriptions/1234/resourceGroups/mcrg/providers/Microsoft.ManagedIdentity/userAssignedIdentities/envid1", "/subscriptions/1234/resourceGroups/mcrg/providers/Microsoft.ManagedIdentity/userAssignedIdentities/envid2"},
ProvisionMode: lo.ToPtr("bootstrappingclient"),
NodeBootstrappingServerURL: lo.ToPtr("https://nodebootstrapping-server-url"),
VnetGUID: lo.ToPtr("a519e60a-cac0-40b2-b883-084477fe6f5c"),
}))
})
})
Context("Validation", func() {
It("should fail when vnet guid is not a uuid", func() {
errMsg := "vnet-guid null is malformed"
err := opts.Parse(
fs,
"--cluster-name", "my-name",
"--cluster-endpoint", "https://karpenter-000000000000.hcp.westus2.staging.azmk8s.io",
"--kubelet-bootstrap-token", "flag-bootstrap-token",
"--ssh-public-key", "flag-ssh-public-key",
"--vm-memory-overhead-percent", "-0.01",
"--network-plugin", "azure",
"--network-plugin-mode", "overlay",
"--vnet-guid", "null", // sometimes output of jq can produce null or some other data, we should enforce that the vnet guid passed in at least looks like a uuid
)
Expect(err).To(MatchError(ContainSubstring(errMsg)))
})

It("should fail when vnet guid is empty for azure cni with overlay clusters", func() {
errMsg := "vnet-guid cannot be empty for AzureCNI clusters with networkPluginMode overlay"
err := opts.Parse(
fs,
"--cluster-name", "my-name",
"--cluster-endpoint", "https://karpenter-000000000000.hcp.westus2.staging.azmk8s.io",
"--kubelet-bootstrap-token", "flag-bootstrap-token",
"--ssh-public-key", "flag-ssh-public-key",
"--vm-memory-overhead-percent", "-0.01",
"--network-plugin", "azure",
"--network-plugin-mode", "overlay",
)
Expect(err).To(MatchError(ContainSubstring(errMsg)))
})
It("should fail when network-plugin-mode is invalid", func() {
typo := "overlaay"
errMsg := fmt.Sprintf("network-plugin-mode %v is invalid. network-plugin-mode must equal 'overlay' or ''", typo)
Expand Down Expand Up @@ -263,6 +296,7 @@ var _ = Describe("Options", func() {
"--ssh-public-key", "flag-ssh-public-key",
"--vnet-subnet-id", "/subscriptions/12345678-1234-1234-1234-123456789012/resourceGroups/sillygeese/providers/Microsoft.Network/virtualNetworks/karpentervnet/subnets/karpentersub",
"--network-plugin", "azure",
"--network-plugin-mode", "",
)
Expect(err).ToNot(HaveOccurred())
})
Expand All @@ -279,6 +313,21 @@ var _ = Describe("Options", func() {
)
Expect(err).ToNot(HaveOccurred())
})
It("should succeed when azure-cni with overlay is configured with the right options", func() {
err := opts.Parse(
fs,
"--cluster-name", "my-name",
"--cluster-endpoint", "https://karpenter-000000000000.hcp.westus2.staging.azmk8s.io",
"--kubelet-bootstrap-token", "flag-bootstrap-token",
"--ssh-public-key", "flag-ssh-public-key",
"--vnet-subnet-id", "/subscriptions/12345678-1234-1234-1234-123456789012/resourceGroups/sillygeese/providers/Microsoft.Network/virtualNetworks/karpentervnet/subnets/karpentersub",
"--network-plugin", "azure",
"--network-plugin-mode", "overlay",
"--vnet-guid", "a519e60a-cac0-40b2-b883-084477fe6f5c",
)
Expect(err).ToNot(HaveOccurred())

})
It("should fail validation when ProvisionMode is not valid", func() {
err := opts.Parse(
fs,
Expand Down
2 changes: 1 addition & 1 deletion pkg/providers/instancetype/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ var _ = Describe("InstanceType Provider", func() {
Expect(decodedString).To(SatisfyAll(
ContainSubstring("kubernetes.azure.com/ebpf-dataplane=cilium"),
ContainSubstring("kubernetes.azure.com/network-subnet=karpentersub"),
ContainSubstring("kubernetes.azure.com/nodenetwork-vnetguid=test-vnet-guid"),
ContainSubstring("kubernetes.azure.com/nodenetwork-vnetguid=a519e60a-cac0-40b2-b883-084477fe6f5c"),
ContainSubstring("kubernetes.azure.com/podnetwork-type=overlay"),
))
})
Expand Down
2 changes: 1 addition & 1 deletion pkg/test/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func Options(overrides ...OptionsFields) *azoptions.Options {
NetworkPlugin: lo.FromPtrOr(options.NetworkPlugin, "azure"),
NetworkPluginMode: lo.FromPtrOr(options.NetworkPluginMode, "overlay"),
NetworkPolicy: lo.FromPtrOr(options.NetworkPolicy, "cilium"),
VnetGUID: lo.FromPtrOr(options.VnetGUID, "test-vnet-guid"),
VnetGUID: lo.FromPtrOr(options.VnetGUID, "a519e60a-cac0-40b2-b883-084477fe6f5c"),
NetworkDataplane: lo.FromPtrOr(options.NetworkDataplane, "cilium"),
VMMemoryOverheadPercent: lo.FromPtrOr(options.VMMemoryOverheadPercent, 0.075),
NodeIdentities: options.NodeIdentities,
Expand Down