Skip to content

Commit 6d116a2

Browse files
Address comments
1 parent e4e92ca commit 6d116a2

File tree

7 files changed

+144
-136
lines changed

7 files changed

+144
-136
lines changed

cns/fakes/imdsclientfake.go

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,20 @@ func (m *MockIMDSClient) GetVMUniqueID(ctx context.Context) (string, error) {
5858
return "55b8499d-9b42-4f85-843f-24ff69f4a643", nil
5959
}
6060

61-
func (m *MockIMDSClient) GetNCVersionsFromIMDS(ctx context.Context) (map[string]string, error) {
61+
func (m *MockIMDSClient) GetNCVersions(ctx context.Context) ([]imds.NetworkInterface, error) {
6262
if ctx.Value(SimulateError) != nil {
6363
return nil, imds.ErrUnexpectedStatusCode
6464
}
6565

6666
// Return some mock NC versions for testing
67-
return map[string]string{
68-
"nc1": "1",
69-
"nc2": "2",
67+
return []imds.NetworkInterface{
68+
{
69+
InterfaceCompartmentID: "nc1",
70+
InterfaceCompartmentVersion: "1",
71+
},
72+
{
73+
InterfaceCompartmentID: "nc2",
74+
InterfaceCompartmentVersion: "2",
75+
},
7076
}, nil
7177
}

cns/imds/client.go

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ const (
5353
metadataHeaderValue = "true"
5454
defaultRetryAttempts = 3
5555
defaultIMDSEndpoint = "http://169.254.169.254"
56-
ncVersion = "ncVersion"
5756
)
5857

5958
var (
@@ -104,7 +103,7 @@ func (c *Client) GetVMUniqueID(ctx context.Context) (string, error) {
104103
return vmUniqueID, nil
105104
}
106105

107-
func (c *Client) GetNCVersionsFromIMDS(ctx context.Context) (map[string]string, error) {
106+
func (c *Client) GetNCVersions(ctx context.Context) ([]NetworkInterface, error) {
108107
var networkData NetworkMetadata
109108
err := retry.Do(func() error {
110109
networkMetadata, err := c.getInstanceMetadata(ctx, imdsNetworkPath)
@@ -125,23 +124,10 @@ func (c *Client) GetNCVersionsFromIMDS(ctx context.Context) (map[string]string,
125124
return nil
126125
}, retry.Context(ctx), retry.Attempts(c.config.retryAttempts), retry.DelayType(retry.BackOffDelay))
127126
if err != nil {
128-
return nil, err
127+
return nil, errors.Wrap(err, "external call failed")
129128
}
130129

131-
ncVersions := make(map[string]string)
132-
for _, iface := range networkData.Interface {
133-
// IMDS only returns compartment fields (interfaceCompartmentId, interfaceCompartmentVersion)
134-
// We map these to NC ID and NC version concepts
135-
// Standard fields (ncId, ncVersion) are ignored even if present
136-
ncId := iface.InterfaceCompartmentId
137-
ncVersion := iface.InterfaceCompartmentVersion
138-
139-
if ncId != "" {
140-
ncVersions[ncId] = ncVersion
141-
}
142-
}
143-
144-
return ncVersions, nil
130+
return networkData.Interface, nil
145131
}
146132

147133
func (c *Client) getInstanceMetadata(ctx context.Context, imdsComputePath string) (map[string]any, error) {
@@ -178,9 +164,8 @@ func (c *Client) getInstanceMetadata(ctx context.Context, imdsComputePath string
178164

179165
// NetworkInterface represents a network interface from IMDS
180166
type NetworkInterface struct {
181-
MacAddress string `json:"macAddress"`
182167
// IMDS only returns compartment fields - these are mapped to NC ID and NC version concepts
183-
InterfaceCompartmentId string `json:"interfaceCompartmentId,omitempty"`
168+
InterfaceCompartmentID string `json:"interfaceCompartmentID,omitempty"`
184169
InterfaceCompartmentVersion string `json:"interfaceCompartmentVersion,omitempty"`
185170
}
186171

cns/imds/client_test.go

Lines changed: 61 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -101,21 +101,19 @@ func TestInvalidVMUniqueID(t *testing.T) {
101101
require.Equal(t, "", vmUniqueID)
102102
}
103103

104-
func TestGetNCVersionsFromIMDS(t *testing.T) {
104+
func TestGetNCVersions(t *testing.T) {
105105
networkMetadata := []byte(`{
106-
"interface": [
107-
{
108-
"macAddress": "00:0D:3A:12:34:56",
109-
"interfaceCompartmentVersion": "1",
110-
"interfaceCompartmentId": "nc-12345-67890"
111-
},
112-
{
113-
"macAddress": "00:0D:3A:CD:EF:12",
114-
"interfaceCompartmentVersion": "",
115-
"interfaceCompartmentId": "nc-abcdef-123456"
116-
}
117-
]
118-
}`)
106+
"interface": [
107+
{
108+
"interfaceCompartmentVersion": "1",
109+
"interfaceCompartmentID": "nc-12345-67890"
110+
},
111+
{
112+
"interfaceCompartmentVersion": "1",
113+
"interfaceCompartmentID": ""
114+
}
115+
]
116+
}`)
119117

120118
mockIMDSServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
121119
// request header "Metadata: true" must be present
@@ -133,80 +131,87 @@ func TestGetNCVersionsFromIMDS(t *testing.T) {
133131

134132
w.WriteHeader(http.StatusOK)
135133
_, writeErr := w.Write(networkMetadata)
136-
require.NoError(t, writeErr, "error writing response")
134+
if writeErr != nil {
135+
t.Errorf("error writing response: %v", writeErr)
136+
return
137+
}
137138
}))
138139
defer mockIMDSServer.Close()
139140

140141
imdsClient := imds.NewClient(imds.Endpoint(mockIMDSServer.URL))
141-
ncVersions, err := imdsClient.GetNCVersionsFromIMDS(context.Background())
142+
interfaces, err := imdsClient.GetNCVersions(context.Background())
142143
require.NoError(t, err, "error querying testserver")
143144

144-
expectedNCVersions := map[string]string{
145-
"nc-12345-67890": "1",
146-
"nc-abcdef-123456": "", // empty version
147-
}
148-
require.Equal(t, expectedNCVersions, ncVersions)
145+
// Verify we got the expected interfaces
146+
require.Len(t, interfaces, 2, "expected 2 interfaces")
147+
148+
// Check first interface
149+
assert.Equal(t, "nc-12345-67890", interfaces[0].InterfaceCompartmentID)
150+
assert.Equal(t, "1", interfaces[0].InterfaceCompartmentVersion)
151+
152+
// Check second interface
153+
assert.Equal(t, "", interfaces[1].InterfaceCompartmentID)
154+
assert.Equal(t, "1", interfaces[1].InterfaceCompartmentVersion)
149155
}
150156

151-
func TestGetNCVersionsFromIMDSInvalidEndpoint(t *testing.T) {
157+
func TestGetNCVersionsInvalidEndpoint(t *testing.T) {
152158
imdsClient := imds.NewClient(imds.Endpoint(string([]byte{0x7f})), imds.RetryAttempts(1))
153-
_, err := imdsClient.GetNCVersionsFromIMDS(context.Background())
159+
_, err := imdsClient.GetNCVersions(context.Background())
154160
require.Error(t, err, "expected invalid path")
155161
}
156162

157-
func TestGetNCVersionsFromIMDSInvalidJSON(t *testing.T) {
158-
mockIMDSServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
163+
func TestGetNCVersionsInvalidJSON(t *testing.T) {
164+
mockIMDSServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
159165
w.WriteHeader(http.StatusOK)
160166
_, err := w.Write([]byte("not json"))
161-
require.NoError(t, err)
167+
if err != nil {
168+
t.Errorf("error writing response: %v", err)
169+
return
170+
}
162171
}))
163172
defer mockIMDSServer.Close()
164173

165174
imdsClient := imds.NewClient(imds.Endpoint(mockIMDSServer.URL), imds.RetryAttempts(1))
166-
_, err := imdsClient.GetNCVersionsFromIMDS(context.Background())
175+
_, err := imdsClient.GetNCVersions(context.Background())
167176
require.Error(t, err, "expected json decoding error")
168177
}
169178

170-
func TestGetNCVersionsFromIMDSNoNCIDs(t *testing.T) {
179+
func TestGetNCVersionsNoNCIDs(t *testing.T) {
171180
networkMetadataNoNC := []byte(`{
172-
"interface": [
173-
{
174-
"macAddress": "00:0D:3A:12:34:56",
175-
"ipv4": {
176-
"ipAddress": [
177-
{
178-
"privateIpAddress": "10.0.0.4",
179-
"publicIpAddress": ""
180-
}
181-
]
182-
}
183-
},
184-
{
185-
"macAddress": "00:0D:3A:78:90:AB",
186-
"ipv4": {
187-
"ipAddress": [
188-
{
189-
"privateIpAddress": "10.0.1.4",
190-
"publicIpAddress": ""
191-
}
192-
]
193-
}
194-
}
195-
]
196-
}`)
181+
"interface": [
182+
{
183+
"ipv4": {
184+
"ipAddress": [
185+
{
186+
"privateIpAddress": "10.0.0.4",
187+
"publicIpAddress": ""
188+
}
189+
]
190+
}
191+
}
192+
]
193+
}`)
197194

198195
mockIMDSServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
199196
metadataHeader := r.Header.Get("Metadata")
200197
assert.Equal(t, "true", metadataHeader)
201198

202199
w.WriteHeader(http.StatusOK)
203200
_, writeErr := w.Write(networkMetadataNoNC)
204-
require.NoError(t, writeErr, "error writing response")
201+
if writeErr != nil {
202+
t.Errorf("error writing response: %v", writeErr)
203+
return
204+
}
205205
}))
206206
defer mockIMDSServer.Close()
207207

208208
imdsClient := imds.NewClient(imds.Endpoint(mockIMDSServer.URL))
209-
ncVersions, err := imdsClient.GetNCVersionsFromIMDS(context.Background())
209+
interfaces, err := imdsClient.GetNCVersions(context.Background())
210210
require.NoError(t, err, "error querying testserver")
211-
require.Empty(t, ncVersions, "expected empty NC versions map when no NC IDs present")
211+
212+
// Verify we got interfaces but they don't have compartment IDs
213+
require.Len(t, interfaces, 1, "expected 1 interface")
214+
215+
// Check that interfaces don't have compartment IDs
216+
assert.Equal(t, "", interfaces[0].InterfaceCompartmentID)
212217
}

0 commit comments

Comments
 (0)