Skip to content

Commit

Permalink
Fix comments and linter errors
Browse files Browse the repository at this point in the history
 - fix kube-vip validation
 - fix kube-vip add to ignore node list
 - add unit-test
  • Loading branch information
adiantum committed Dec 18, 2024
1 parent 1646054 commit 43fc362
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 19 deletions.
21 changes: 2 additions & 19 deletions pkg/providers/nutanix/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -601,21 +601,6 @@ func compareIP(ip1, ip2 net.IP) (int, error) {
return 0, nil

Check warning on line 601 in pkg/providers/nutanix/template.go

View check run for this annotation

Codecov / codecov/patch

pkg/providers/nutanix/template.go#L601

Added line #L601 was not covered by tests
}

func addKubeVipToIgnoredNodeIPsList(clusterSpec *cluster.Spec, result []string) []string {
kubeVipStr := clusterSpec.Cluster.Spec.ControlPlaneConfiguration.Endpoint.Host
if kubeVipStr != "" {
kubeVip, err := net.ResolveIPAddr("ip", kubeVipStr)
if err != nil {
// log error and continue
log.Printf("error resolving kube-vip IP address %s: %v", kubeVipStr, err)
} else {
result = append(result, kubeVip.IP.String())
}
}

return result
}

func addCIDRToIgnoredNodeIPsList(cidr string, result []string) []string {
ip, ipNet, err := net.ParseCIDR(cidr)
if err != nil {
Expand Down Expand Up @@ -687,10 +672,8 @@ func addIPAddressToIgnoredNodeIPsList(ipAddrStr string, result []string) []strin
}

func generateCcmIgnoredNodeIPsList(clusterSpec *cluster.Spec) []string {
result := make([]string, 0)

// Add kube-vip to the list
result = addKubeVipToIgnoredNodeIPsList(clusterSpec, result)
// Add the kube-vip IP address to the list
result := []string{clusterSpec.Cluster.Spec.ControlPlaneConfiguration.Endpoint.Host}

for _, IPAddrOrRange := range clusterSpec.NutanixDatacenter.Spec.CcmExcludeNodeIPs {
addrOrRange := strings.TrimSpace(IPAddrOrRange)
Expand Down
13 changes: 13 additions & 0 deletions pkg/providers/nutanix/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,15 @@ func NewValidator(clientCache *ClientCache, certValidator crypto.TlsValidator, h
}
}

func (v *Validator) validateControlPlaneIP(ip string) error {
// check if controlPlaneEndpointIp is valid
parsedIP := net.ParseIP(ip)
if parsedIP == nil {
return fmt.Errorf("cluster controlPlaneConfiguration.Endpoint.Host is invalid: %s", ip)
}
return nil
}

// ValidateClusterSpec validates the cluster spec.
func (v *Validator) ValidateClusterSpec(ctx context.Context, spec *cluster.Spec, creds credentials.BasicAuthCredential) error {
logger.Info("ValidateClusterSpec for Nutanix datacenter", "NutanixDatacenter", spec.NutanixDatacenter.Name)
Expand All @@ -61,6 +70,10 @@ func (v *Validator) ValidateClusterSpec(ctx context.Context, spec *cluster.Spec,
return err
}

if err := v.validateControlPlaneIP(spec.Cluster.Spec.ControlPlaneConfiguration.Endpoint.Host); err != nil {
return err
}

Check warning on line 75 in pkg/providers/nutanix/validator.go

View check run for this annotation

Codecov / codecov/patch

pkg/providers/nutanix/validator.go#L74-L75

Added lines #L74 - L75 were not covered by tests

for _, conf := range spec.NutanixMachineConfigs {
if err := v.ValidateMachineConfig(ctx, client, spec.Cluster, conf); err != nil {
return fmt.Errorf("failed to validate machine config: %v", err)
Expand Down
68 changes: 68 additions & 0 deletions pkg/providers/nutanix/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1868,3 +1868,71 @@ func TestValidateMachineConfigFailureDomainsWrongCount(t *testing.T) {
t.Fatalf("validation should not pass: %v", err)
}
}

func TestValidateControlPlaneIP(t *testing.T) {
tests := []struct {
name string
clusterConfig string
setIPfunc func(*cluster.Spec) *cluster.Spec
expectErr bool
}{
{
name: "valid control plane IP",
clusterConfig: "testdata/eksa-cluster.yaml",
setIPfunc: func(clusterSpec *cluster.Spec) *cluster.Spec {
clusterSpec.Cluster.Spec.ControlPlaneConfiguration.Endpoint.Host = "10.199.198.12"
return clusterSpec
},
expectErr: false,
},
{
name: "invalid control plane IP",
clusterConfig: "testdata/eksa-cluster.yaml",
setIPfunc: func(clusterSpec *cluster.Spec) *cluster.Spec {
clusterSpec.Cluster.Spec.ControlPlaneConfiguration.Endpoint.Host = "not-an-ip"
return clusterSpec
},
expectErr: true,
},
}

ctrl := gomock.NewController(t)
mockClient := mocknutanix.NewMockClient(ctrl)
mockClient.EXPECT().GetCurrentLoggedInUser(gomock.Any()).Return(&v3.UserIntentResponse{}, nil).AnyTimes()
mockClient.EXPECT().ListAllCluster(gomock.Any(), gomock.Any()).DoAndReturn(
func(_ context.Context, filter string) (*v3.ClusterListIntentResponse, error) {
return fakeClusterListForDCTest(filter)
},
).AnyTimes()
mockClient.EXPECT().ListAllSubnet(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(
func(_ context.Context, filter string) (*v3.SubnetListIntentResponse, error) {
return fakeSubnetListForDCTest(filter)
},
).AnyTimes()
mockClient.EXPECT().GetSubnet(gomock.Any(), gomock.Eq("2d166190-7759-4dc6-b835-923262d6b497")).Return(nil, nil).AnyTimes()
mockClient.EXPECT().GetCluster(gomock.Any(), gomock.Eq("4d69ca7d-022f-49d1-a454-74535993bda4")).Return(nil, nil).AnyTimes()

mockTLSValidator := mockCrypto.NewMockTlsValidator(ctrl)
mockTLSValidator.EXPECT().ValidateCert(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()

mockTransport := mocknutanix.NewMockRoundTripper(ctrl)
mockTransport.EXPECT().RoundTrip(gomock.Any()).Return(&http.Response{}, nil).AnyTimes()

mockHTTPClient := &http.Client{Transport: mockTransport}
clientCache := &ClientCache{clients: map[string]Client{"test": mockClient}}
validator := NewValidator(clientCache, mockTLSValidator, mockHTTPClient)

// Run tests
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
clusterSpec := test.NewFullClusterSpec(t, tc.clusterConfig)
clusterSpec = tc.setIPfunc(clusterSpec)
err := validator.validateControlPlaneIP(clusterSpec.Cluster.Spec.ControlPlaneConfiguration.Endpoint.Host)
if tc.expectErr {
assert.Error(t, err, tc.name)
} else {
assert.NoError(t, err, tc.name)
}
})
}
}

0 comments on commit 43fc362

Please sign in to comment.