Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions cns/NetworkContainerContract.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@
AllowNCToHostCommunication bool
EndpointPolicies []NetworkContainerRequestPolicies
NCStatus v1alpha.NCStatus
SwiftV2PrefixOnNic bool // Indicates if is swiftv2 nc, PrefixOnNic scenario (isSwiftV2 && nc.Type == VNETBlock)

Check failure on line 131 in cns/NetworkContainerContract.go

View workflow job for this annotation

GitHub Actions / Lint (ubuntu-latest)

File is not properly formatted (gci)

Check failure on line 131 in cns/NetworkContainerContract.go

View workflow job for this annotation

GitHub Actions / Lint (windows-latest)

File is not properly formatted (gci)
NetworkInterfaceInfo NetworkInterfaceInfo //nolint // introducing new field for backendnic, to be used later by cni code
}

Expand Down
1 change: 1 addition & 0 deletions cns/kubecontroller/nodenetworkconfig/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
NetworkContainerid: nc.ID,
NetworkContainerType: cns.Docker,
Version: strconv.FormatInt(nc.Version, 10), //nolint:gomnd // it's decimal
SwiftV2PrefixOnNic: false, // Dynamic NCs don't use SwiftV2 PrefixOnNic

Check failure on line 65 in cns/kubecontroller/nodenetworkconfig/conversion.go

View workflow job for this annotation

GitHub Actions / Lint (ubuntu-latest)

File is not properly formatted (gci)

Check failure on line 65 in cns/kubecontroller/nodenetworkconfig/conversion.go

View workflow job for this annotation

GitHub Actions / Lint (windows-latest)

File is not properly formatted (gci)
IPConfiguration: cns.IPConfiguration{
IPSubnet: subnet,
GatewayIPAddress: nc.DefaultGateway,
Expand Down
1 change: 1 addition & 0 deletions cns/kubecontroller/nodenetworkconfig/conversion_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPre
GatewayIPv6Address: nc.DefaultGatewayV6,
},
NCStatus: nc.Status,
SwiftV2PrefixOnNic: isSwiftV2 && nc.Type == v1alpha.VNETBlock,
NetworkInterfaceInfo: cns.NetworkInterfaceInfo{
MACAddress: nc.MacAddress,
},
Expand Down
23 changes: 21 additions & 2 deletions cns/restserver/internalapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"strconv"
"strings"
"time"
"runtime"

Check failure on line 18 in cns/restserver/internalapi.go

View workflow job for this annotation

GitHub Actions / Lint (ubuntu-latest)

File is not properly formatted (gci)

Check failure on line 18 in cns/restserver/internalapi.go

View workflow job for this annotation

GitHub Actions / Lint (windows-latest)

File is not properly formatted (gci)

"github.com/Azure/azure-container-networking/cns"
"github.com/Azure/azure-container-networking/cns/logger"
Expand Down Expand Up @@ -229,7 +230,7 @@
}

// Get IMDS NC versions for delegated NIC scenarios
imdsNCVersions, err := service.GetIMDSNCs(ctx)
imdsNCVersions, err := service.getIMDSNCs(ctx)
if err != nil {
// If any of the NMA API check calls, imds calls fails assume that nma build doesn't have the latest changes and create empty map
imdsNCVersions = make(map[string]string)
Expand Down Expand Up @@ -685,7 +686,7 @@
}

// GetIMDSNCs gets NC versions from IMDS and returns them as a map
func (service *HTTPRestService) GetIMDSNCs(ctx context.Context) (map[string]string, error) {
func (service *HTTPRestService) getIMDSNCs(ctx context.Context) (map[string]string, error) {
imdsClient := service.imdsClient
if imdsClient == nil {
//nolint:staticcheck // SA1019: suppress deprecated logger.Printf usage. Todo: legacy logger usage is consistent in cns repo. Migrates when all logger usage is migrated
Expand Down Expand Up @@ -717,8 +718,26 @@

if ncID != "" {
ncs[ncID] = PrefixOnNicNCVersion // for prefix on nic version scenario nc version is 1
} else if runtime.GOOS == "windows" && service.isPrefixonNicSwiftV2() {
err := service.setPrefixOnNICRegistry(true, iface.MacAddress.String())
if err != nil {
logger.Debugf("failed to add PrefixOnNic keys to Windows registry: %w", err)

Check failure on line 724 in cns/restserver/internalapi.go

View workflow job for this annotation

GitHub Actions / Lint (ubuntu-latest)

SA1019: logger.Debugf is deprecated: The global logger is deprecated. Migrate to zap using the cns/logger/v2 package and pass the logger instead. (staticcheck)
}
}
}

return ncs, nil
}

// Check whether NC is SwiftV2 NIC associated NC and prefix on nic is enabled
func (service *HTTPRestService) isPrefixonNicSwiftV2() bool {
for _, containerStatus := range service.state.ContainerStatus {

Check failure on line 734 in cns/restserver/internalapi.go

View workflow job for this annotation

GitHub Actions / Lint (ubuntu-latest)

rangeValCopy: each iteration copies 504 bytes (consider pointers or indexing) (gocritic)

Check failure on line 734 in cns/restserver/internalapi.go

View workflow job for this annotation

GitHub Actions / Lint (windows-latest)

rangeValCopy: each iteration copies 504 bytes (consider pointers or indexing) (gocritic)
req := containerStatus.CreateNetworkContainerRequest

//
if req.SwiftV2PrefixOnNic {
return true
}
}
return false
}
9 changes: 9 additions & 0 deletions cns/restserver/internalapi_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,3 +181,12 @@
func (service *HTTPRestService) setVFForAccelnetNICs() error {
return nil
}

func (service *HTTPRestService) setPrefixOnNICRegistry(enabled bool, infraNicMacAddress string) error {
// Assigning parameters to '_' to avoid unused parameter linting errors.

Check failure on line 186 in cns/restserver/internalapi_linux.go

View workflow job for this annotation

GitHub Actions / Lint (ubuntu-latest)

File is not properly formatted (gci)
// These parameters are only used in the Windows implementation.
_ = enabled
_ = infraNicMacAddress
logger.Printf("[setPrefixOnNicEnabled winDebug] No-op on Linux platform")

Check failure on line 190 in cns/restserver/internalapi_linux.go

View workflow job for this annotation

GitHub Actions / Lint (ubuntu-latest)

SA1019: logger.Printf is deprecated: The global logger is deprecated. Migrate to zap using the cns/logger/v2 package and pass the logger instead. (staticcheck)
return nil
}
133 changes: 133 additions & 0 deletions cns/restserver/internalapi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"sync"
"testing"
"time"
"runtime"

Check failure on line 18 in cns/restserver/internalapi_test.go

View workflow job for this annotation

GitHub Actions / Lint (ubuntu-latest)

File is not properly formatted (gci)

Check failure on line 18 in cns/restserver/internalapi_test.go

View workflow job for this annotation

GitHub Actions / Lint (windows-latest)

File is not properly formatted (gci)

"github.com/Azure/azure-container-networking/cns"
"github.com/Azure/azure-container-networking/cns/common"
Expand Down Expand Up @@ -1680,3 +1681,135 @@
// Return cleanup function
return func() { svc.imdsClient = originalIMDS }
}

// TestSyncHostNCVersionWithWindowsSwiftV2 tests SyncHostNCVersion and verifies it calls Windows SwiftV2 PrefixOnNic scenario
func TestSyncHostNCVersionWithWindowsSwiftV2(t *testing.T) {
svc := getTestService(cns.Kubernetes)

Check failure on line 1687 in cns/restserver/internalapi_test.go

View workflow job for this annotation

GitHub Actions / Lint (ubuntu-latest)

shadow: declaration of "svc" shadows declaration at line 66 (govet)

Check failure on line 1687 in cns/restserver/internalapi_test.go

View workflow job for this annotation

GitHub Actions / Lint (windows-latest)

shadow: declaration of "svc" shadows declaration at line 66 (govet)

// Set up test NCs with different scenarios
regularNCID := "regular-nc-id"
swiftV2NCID := "swift-v2-vnet-block-nc"

// Initialize ContainerStatus map if nil
if svc.state.ContainerStatus == nil {
svc.state.ContainerStatus = make(map[string]containerstatus)
}

// Add a regular NC
svc.state.ContainerStatus[regularNCID] = containerstatus{
ID: regularNCID,
CreateNetworkContainerRequest: cns.CreateNetworkContainerRequest{
NetworkContainerid: regularNCID,
SwiftV2PrefixOnNic: false,
NetworkContainerType: cns.Docker,
Version: "2",
},
HostVersion: "1",
}

// Add a SwiftV2 VNETBlock NC that should trigger Windows registry operations
svc.state.ContainerStatus[swiftV2NCID] = containerstatus{
ID: swiftV2NCID,
CreateNetworkContainerRequest: cns.CreateNetworkContainerRequest{
NetworkContainerid: swiftV2NCID,
SwiftV2PrefixOnNic: true,
NetworkContainerType: cns.Docker,
Version: "2",
},
HostVersion: "1",
}

// Set up mock NMAgent with NC versions
mockNMA := &fakes.NMAgentClientFake{}
mockNMA.GetNCVersionListF = func(ctx context.Context) (nma.NCVersionList, error) {

Check failure on line 1724 in cns/restserver/internalapi_test.go

View workflow job for this annotation

GitHub Actions / Lint (ubuntu-latest)

unused-parameter: parameter 'ctx' seems to be unused, consider removing or renaming it as _ (revive)

Check failure on line 1724 in cns/restserver/internalapi_test.go

View workflow job for this annotation

GitHub Actions / Lint (windows-latest)

unused-parameter: parameter 'ctx' seems to be unused, consider removing or renaming it as _ (revive)
return nma.NCVersionList{
Containers: []nma.NCVersion{
{
NetworkContainerID: regularNCID,
Version: "2",
},
{
NetworkContainerID: swiftV2NCID,
Version: "2",
},
},
}, nil
}
svc.nma = mockNMA

// Set up mock IMDS client for Windows SwiftV2 scenario
mac1, _ := net.ParseMAC("AA:BB:CC:DD:EE:FF")
mac2, _ := net.ParseMAC("11:22:33:44:55:66")

interfaceMap := map[string]imds.NetworkInterface{
"interface1": {
InterfaceCompartmentID: "", // Empty for Windows condition
MacAddress: imds.HardwareAddr(mac1),
},
"interface2": {
InterfaceCompartmentID: "nc-with-compartment-id",
MacAddress: imds.HardwareAddr(mac2),
},
}
mockIMDS := &mockIMDSAdapter{
mock: &struct {
networkInterfaces func(_ context.Context) ([]imds.NetworkInterface, error)
imdsVersions func(_ context.Context) (*imds.APIVersionsResponse, error)
}{
networkInterfaces: func(_ context.Context) ([]imds.NetworkInterface, error) {
var interfaces []imds.NetworkInterface
for _, iface := range interfaceMap {
interfaces = append(interfaces, iface)
}
return interfaces, nil
},
imdsVersions: func(_ context.Context) (*imds.APIVersionsResponse, error) {
return &imds.APIVersionsResponse{
APIVersions: []string{expectedIMDSAPIVersion},
}, nil
},
},
}

// Replace the IMDS client
originalIMDS := svc.imdsClient
svc.imdsClient = mockIMDS
defer func() { svc.imdsClient = originalIMDS }()

// Verify preconditions
assert.True(t, svc.isPrefixonNicSwiftV2(), "isPrefixonNicSwiftV2() should return true")

ctx := context.Background()
svc.SyncHostNCVersion(ctx, cns.CRD)

// Verify that NC versions were updated
updatedRegularNC := svc.state.ContainerStatus[regularNCID]
updatedSwiftV2NC := svc.state.ContainerStatus[swiftV2NCID]

assert.Equal(t, "2", updatedRegularNC.HostVersion, "Regular NC host version should be updated to 2")
assert.Equal(t, "2", updatedSwiftV2NC.HostVersion, "SwiftV2 NC host version should be updated to 2")

imdsNCs, err := svc.getIMDSNCs(ctx)
assert.NoError(t, err, "getIMDSNCs should not return error")

// Verify IMDS results
assert.Contains(t, imdsNCs, "nc-with-compartment-id", "NC with compartment ID should be in results")
assert.Equal(t, PrefixOnNicNCVersion, imdsNCs["nc-with-compartment-id"], "NC should have expected version")

// Log the conditions that would trigger Windows registry operations
isWindows := runtime.GOOS == "windows"
hasSwiftV2PrefixOnNic := svc.isPrefixonNicSwiftV2()

t.Logf("Windows SwiftV2 PrefixOnNic conditions: (runtime.GOOS == 'windows' && service.isPrefixonNicSwiftV2()): %t",
isWindows && hasSwiftV2PrefixOnNic)


// Test with no SwiftV2 NCs
delete(svc.state.ContainerStatus, swiftV2NCID)
assert.False(t, svc.isPrefixonNicSwiftV2(), "isPrefixonNicSwiftV2() should return false without SwiftV2 NCs")

// Call getIMDSNCs again to verify condition is not triggered
_, err2 := svc.getIMDSNCs(ctx)
assert.NoError(t, err2, "getIMDSNCs should not return error")
}

76 changes: 75 additions & 1 deletion cns/restserver/internalapi_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,20 @@
"time"

"github.com/Azure/azure-container-networking/cns"
"github.com/Azure/azure-container-networking/cns/logger"
"github.com/Azure/azure-container-networking/cns/types"
"github.com/Microsoft/hcsshim"
"github.com/pkg/errors"
"golang.org/x/sys/windows/registry"
)

const (
// timeout for powershell command to return the interfaces list
pwshTimeout = 120 * time.Second
pwshTimeout = 120 * time.Second

Check failure on line 18 in cns/restserver/internalapi_windows.go

View workflow job for this annotation

GitHub Actions / Lint (windows-latest)

File is not properly formatted (gci)
hnsRegistryPath = `SYSTEM\CurrentControlSet\Services\HNS\wcna_state\config`
prefixOnNicRegistryPath = `SYSTEM\CurrentControlSet\Services\HNS\wcna_state\config\PrefixOnNic`
infraNicIfName = "eth0"
enableSNAT = false
)

var errUnsupportedAPI = errors.New("unsupported api")
Expand Down Expand Up @@ -75,3 +81,71 @@
}
return macAddress, nil
}

func (service *HTTPRestService) enablePrefixOnNic(enabled bool) error {
return service.setRegistryValue(prefixOnNicRegistryPath, "enabled", enabled)
}

func (service *HTTPRestService) setInfraNicMacAddress(macAddress string) error {
return service.setRegistryValue(prefixOnNicRegistryPath, "infra_nic_mac_address", macAddress)
}

func (service *HTTPRestService) setInfraNicIfName(ifName string) error {
return service.setRegistryValue(prefixOnNicRegistryPath, "infra_nic_ifname", ifName)
}

func (service *HTTPRestService) setEnableSNAT(enabled bool) error {
return service.setRegistryValue(hnsRegistryPath, "EnableSNAT", enabled)
}

func (service *HTTPRestService) setPrefixOnNICRegistry(enablePrefixOnNic bool, infraNicMacAddress string) error {
if err := service.enablePrefixOnNic(enablePrefixOnNic); err != nil {
return fmt.Errorf("failed to set enablePrefixOnNic key to windows registry: %w", err)
}

if err := service.setInfraNicMacAddress(infraNicMacAddress); err != nil {
return fmt.Errorf("failed to set InfraNicMacAddress key to windows registry: %w", err)
}

if err := service.setInfraNicIfName(infraNicIfName); err != nil {
return fmt.Errorf("failed to set InfraNicIfName key to windows registry: %w", err)
}

if err := service.setEnableSNAT(enableSNAT); err != nil {
return fmt.Errorf("failed to set EnableSNAT key to windows registry: %w", err)
}

return nil
}

func (service *HTTPRestService) setRegistryValue(registryPath, keyName string, value interface{}) error {
key, _, err := registry.CreateKey(registry.LOCAL_MACHINE, registryPath, registry.SET_VALUE)
if err != nil {
return fmt.Errorf("failed to create/open registry key %s: %w", registryPath, err)
}
defer key.Close()

switch v := value.(type) {
case string:
err = key.SetStringValue(keyName, v)
case bool:
dwordValue := uint32(0)
if v {
dwordValue = 1
}
err = key.SetDWordValue(keyName, dwordValue)
case uint32:
err = key.SetDWordValue(keyName, v)
case int:
err = key.SetDWordValue(keyName, uint32(v))

Check failure on line 140 in cns/restserver/internalapi_windows.go

View workflow job for this annotation

GitHub Actions / Lint (windows-latest)

G115: integer overflow conversion int -> uint32 (gosec)
default:
return fmt.Errorf("unsupported value type for registry key %s: %T", keyName, value)

Check failure on line 142 in cns/restserver/internalapi_windows.go

View workflow job for this annotation

GitHub Actions / Lint (windows-latest)

do not define dynamic errors, use wrapped static errors instead: "fmt.Errorf(\"unsupported value type for registry key %s: %T\", keyName, value)" (err113)
}

if err != nil {
return fmt.Errorf("failed to set registry value '%s': %w", keyName, err)
}

logger.Printf("[setRegistryValue] Set %s\\%s = %v", registryPath, keyName, value)
return nil
}
Loading