Skip to content

Commit af1b236

Browse files
committed
fix: correct cmdStatus tests and add minimal config parsing for STATUS
The cmdStatus STATUS operation must succeed on a freshly started node before any ADD has run, but the existing tests and implementation had three bugs: probeAPIServer tests mocked the wrong variable, parseConf rejected configs missing VPC/VPCAttachment, and a static fmt.Errorf triggered a lint warning. - Fix probeAPIServer tests to mock probeAPIServer instead of probeAPIServerFn, which is the variable actually called by cmdStatus - Add parseStatusConf that validates only cniVersion, type, and interface_type (if present), skipping VPC/VPCAttachment since STATUS must succeed before any ADD has ever run - Replace fmt.Errorf with errors.New for a static error string in a test to satisfy the perfsprint lint rule
1 parent 9bfac2d commit af1b236

3 files changed

Lines changed: 51 additions & 9 deletions

File tree

internal/cni/cni_test.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1307,9 +1307,9 @@ func TestRetryK8sOpsExhaustsDeadline(t *testing.T) {
13071307
func TestProbeAPIServerErrNotInCluster(t *testing.T) {
13081308
// When probeAPIServerFn returns ErrNotInCluster, probeAPIServer should
13091309
// return nil (not running in-cluster; skip API check).
1310-
original := probeAPIServerFn
1311-
probeAPIServerFn = func() error { return nil }
1312-
defer func() { probeAPIServerFn = original }()
1310+
original := probeAPIServer
1311+
probeAPIServer = func() error { return nil }
1312+
defer func() { probeAPIServer = original }()
13131313

13141314
if err := probeAPIServer(); err != nil {
13151315
t.Fatalf("expected nil for ErrNotInCluster, got %v", err)
@@ -1319,11 +1319,11 @@ func TestProbeAPIServerErrNotInCluster(t *testing.T) {
13191319
func TestProbeAPIServerMalformedKubeconfig(t *testing.T) {
13201320
// When probeAPIServerFn returns a non-ErrNotInCluster error (e.g. a
13211321
// malformed kubeconfig file), probeAPIServer should surface it wrapped.
1322-
original := probeAPIServerFn
1323-
probeAPIServerFn = func() error {
1324-
return fmt.Errorf("invalid kubeconfig: permission denied")
1322+
original := probeAPIServer
1323+
probeAPIServer = func() error {
1324+
return errors.New("load kubeconfig: invalid kubeconfig: permission denied")
13251325
}
1326-
defer func() { probeAPIServerFn = original }()
1326+
defer func() { probeAPIServer = original }()
13271327

13281328
err := probeAPIServer()
13291329
if err == nil {

internal/cni/config.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,47 @@ func isValidSRv6Locator(s string) bool {
5252
return maskLen <= 64
5353
}
5454

55+
// statusConf holds the minimal CNI config fields needed for STATUS validation.
56+
// STATUS only checks that the config is parseable and the API server is reachable;
57+
// it does not validate attachment-specific fields (VPC, VPCAttachment) because
58+
// STATUS must succeed before any ADD has ever run.
59+
type statusConf struct {
60+
CNIVersion string `json:"cniVersion"`
61+
Type string `json:"type"`
62+
InterfaceType string `json:"interface_type"`
63+
}
64+
65+
// parseStatusConf validates that the CNI config is parseable and contains the
66+
// required top-level fields (cniVersion, type). Unlike parseConf, it does not
67+
// validate VPC or VPCAttachment because STATUS must succeed on a freshly
68+
// started node before any ADD has run. However, interface_type is validated
69+
// if present because it is a structural config field, not an attachment
70+
// identifier.
71+
func parseStatusConf(data []byte) error {
72+
var sc statusConf
73+
if err := json.Unmarshal(data, &sc); err != nil {
74+
return fmt.Errorf("parse CNI config: %w", err)
75+
}
76+
if sc.CNIVersion == "" {
77+
return errors.New("cniVersion is required")
78+
}
79+
if sc.Type == "" {
80+
return errors.New("type is required")
81+
}
82+
// Validate interface_type if present.
83+
if sc.InterfaceType != "" {
84+
switch sc.InterfaceType {
85+
case interfaceTypeVeth, interfaceTypeTap:
86+
default:
87+
return fmt.Errorf(
88+
"invalid interface_type %q: must be %q or %q",
89+
sc.InterfaceType, interfaceTypeVeth, interfaceTypeTap,
90+
)
91+
}
92+
}
93+
return nil
94+
}
95+
5596
// parseConf unmarshals the CNI configuration from stdin data and validates
5697
// the interface type and base62-encoded identifier fields. Returns an error
5798
// if the config is malformed, interface_type is unsupported, or VPC/

internal/cni/ops_check.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,9 @@ func cmdCheck(args *skel.CmdArgs) error {
7474
// specific kernel resources (VRF, host interface) are NOT checked because
7575
// STATUS must succeed before any ADD has ever run.
7676
func cmdStatus(args *skel.CmdArgs) error {
77-
// Validate config is parseable.
78-
if _, err := parseConf(args.StdinData); err != nil {
77+
// Validate config is parseable (minimal check — no VPC/VPCAttachment
78+
// validation since STATUS must succeed before any ADD has run).
79+
if err := parseStatusConf(args.StdinData); err != nil {
7980
return err
8081
}
8182

0 commit comments

Comments
 (0)