Skip to content

Commit 3424d94

Browse files
sjbermansjberman
authored
Use UUID instead of IP address for tracking agent (#4470)
Problem: In some cases, a Pod's IP address may not actually be the IP of the Pod. It could be the IP of the node it's running on. This makes verification and tracking difficult to impossible. Solution: Use the UUID of the container instead of the IP address, and loosen the validation to not care about the address that connects to the control plane. Also needed to fix the agent's ability to identify us as a container, because otherwise the UUID was of the node, not the container. This involved reverting a previous fix that was added after removing the default service account directory, which agent looked for.
1 parent c2f7394 commit 3424d94

File tree

12 files changed

+56
-129
lines changed

12 files changed

+56
-129
lines changed

‎build/Dockerfile.nginx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ COPY ${NGINX_CONF_DIR}/nginx.conf /etc/nginx/nginx.conf
3131
COPY ${NGINX_CONF_DIR}/grpc-error-locations.conf /etc/nginx/grpc-error-locations.conf
3232
COPY ${NGINX_CONF_DIR}/grpc-error-pages.conf /etc/nginx/grpc-error-pages.conf
3333

34+
# Create empty /run/.containerenv file so agent can identify that it's running in a container
35+
RUN mkdir -p /run && touch /run/.containerenv
36+
3437
RUN chown -R 101:1001 /etc/nginx /var/cache/nginx
3538

3639
LABEL org.nginx.ngf.image.build.agent="${BUILD_AGENT}"

‎build/Dockerfile.nginxplus‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ COPY ${NGINX_CONF_DIR}/nginx-plus.conf /etc/nginx/nginx.conf
3434
COPY ${NGINX_CONF_DIR}/grpc-error-locations.conf /etc/nginx/grpc-error-locations.conf
3535
COPY ${NGINX_CONF_DIR}/grpc-error-pages.conf /etc/nginx/grpc-error-pages.conf
3636

37+
# Create empty /run/.containerenv file so agent can identify that it's running in a container
38+
RUN mkdir -p /run && touch /run/.containerenv
39+
3740
RUN chown -R 101:1001 /etc/nginx /var/cache/nginx /var/lib/nginx
3841

3942
USER 101:1001

‎build/ubi/Dockerfile.nginx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,9 @@ COPY ${NGINX_CONF_DIR}/nginx.conf /etc/nginx/nginx.conf
6363
COPY ${NGINX_CONF_DIR}/grpc-error-locations.conf /etc/nginx/grpc-error-locations.conf
6464
COPY ${NGINX_CONF_DIR}/grpc-error-pages.conf /etc/nginx/grpc-error-pages.conf
6565

66+
# Create empty /run/.containerenv file so agent can identify that it's running in a container
67+
RUN mkdir -p /run && touch /run/.containerenv
68+
6669
# Switch to non-root user
6770
USER 101:1001
6871

‎build/ubi/Dockerfile.nginxplus‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,9 @@ COPY ${NGINX_CONF_DIR}/nginx.conf /etc/nginx/nginx.conf
7171
COPY ${NGINX_CONF_DIR}/grpc-error-locations.conf /etc/nginx/grpc-error-locations.conf
7272
COPY ${NGINX_CONF_DIR}/grpc-error-pages.conf /etc/nginx/grpc-error-pages.conf
7373

74+
# Create empty /run/.containerenv file so agent can identify that it's running in a container
75+
RUN mkdir -p /run && touch /run/.containerenv
76+
7477
# Switch to non-root user
7578
USER 101:1001
7679

‎internal/controller/nginx/agent/command.go‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,6 @@ func (cs *commandService) CreateConnection(
8484

8585
resource := req.GetResource()
8686
podName := resource.GetContainerInfo().GetHostname()
87-
if podName == "" {
88-
podName = resource.GetHostInfo().GetHostname()
89-
}
9087
cs.logger.Info(fmt.Sprintf("Creating connection for nginx pod: %s", podName))
9188

9289
owner, _, err := cs.getPodOwner(podName)
@@ -107,7 +104,7 @@ func (cs *commandService) CreateConnection(
107104
PodName: podName,
108105
InstanceID: getNginxInstanceID(resource.GetInstances()),
109106
}
110-
cs.connTracker.Track(gi.IPAddress, conn)
107+
cs.connTracker.Track(gi.UUID, conn)
111108

112109
return &pb.CreateConnectionResponse{
113110
Response: &pb.CommandResponse{
@@ -133,7 +130,7 @@ func (cs *commandService) Subscribe(in pb.CommandService_SubscribeServer) error
133130
if !ok {
134131
return agentgrpc.ErrStatusInvalidConnection
135132
}
136-
defer cs.connTracker.RemoveConnection(gi.IPAddress)
133+
defer cs.connTracker.RemoveConnection(gi.UUID)
137134

138135
// wait for the agent to report itself and nginx
139136
conn, deployment, err := cs.waitForConnection(ctx, gi)
@@ -261,7 +258,7 @@ func (cs *commandService) waitForConnection(
261258
case <-timer.C:
262259
return nil, nil, err
263260
case <-ticker.C:
264-
if conn := cs.connTracker.GetConnection(gi.IPAddress); conn.Ready() {
261+
if conn := cs.connTracker.GetConnection(gi.UUID); conn.Ready() {
265262
// connection has been established, now ensure that the deployment exists in the store
266263
if deployment := cs.nginxDeployments.Get(conn.Parent); deployment != nil {
267264
return &conn, deployment, nil
@@ -575,7 +572,7 @@ func (cs *commandService) UpdateDataPlaneStatus(
575572
return nil, grpcStatus.Errorf(codes.InvalidArgument, "request does not contain nginx instanceID")
576573
}
577574

578-
cs.connTracker.SetInstanceID(gi.IPAddress, instanceID)
575+
cs.connTracker.SetInstanceID(gi.UUID, instanceID)
579576

580577
return &pb.UpdateDataPlaneStatusResponse{}, nil
581578
}

‎internal/controller/nginx/agent/command_test.go‎

Lines changed: 4 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -73,15 +73,15 @@ func createFakeK8sClient(initObjs ...runtime.Object) (client.Client, error) {
7373

7474
func createGrpcContext() context.Context {
7575
return grpcContext.NewGrpcContext(context.Background(), grpcContext.GrpcInfo{
76-
IPAddress: "127.0.0.1",
76+
UUID: "1234567",
7777
})
7878
}
7979

8080
func createGrpcContextWithCancel() (context.Context, context.CancelFunc) {
8181
ctx, cancel := context.WithCancel(context.Background())
8282

8383
return grpcContext.NewGrpcContext(ctx, grpcContext.GrpcInfo{
84-
IPAddress: "127.0.0.1",
84+
UUID: "1234567",
8585
}), cancel
8686
}
8787

@@ -163,32 +163,6 @@ func TestCreateConnection(t *testing.T) {
163163
},
164164
},
165165
},
166-
{
167-
name: "uses regular hostname if container info not set",
168-
ctx: createGrpcContext(),
169-
request: &pb.CreateConnectionRequest{
170-
Resource: &pb.Resource{
171-
Info: &pb.Resource_HostInfo{
172-
HostInfo: &pb.HostInfo{
173-
Hostname: "nginx-pod",
174-
},
175-
},
176-
Instances: []*pb.Instance{
177-
{
178-
InstanceMeta: &pb.InstanceMeta{
179-
InstanceId: "nginx-id",
180-
InstanceType: pb.InstanceMeta_INSTANCE_TYPE_NGINX,
181-
},
182-
},
183-
},
184-
},
185-
},
186-
response: &pb.CreateConnectionResponse{
187-
Response: &pb.CommandResponse{
188-
Status: pb.CommandResponse_COMMAND_STATUS_OK,
189-
},
190-
},
191-
},
192166
{
193167
name: "request is nil",
194168
request: nil,
@@ -268,7 +242,7 @@ func TestCreateConnection(t *testing.T) {
268242
}
269243

270244
key, conn := connTracker.TrackArgsForCall(0)
271-
g.Expect(key).To(Equal("127.0.0.1"))
245+
g.Expect(key).To(Equal("1234567"))
272246
g.Expect(conn).To(Equal(expConn))
273247
})
274248
}
@@ -1062,7 +1036,7 @@ func TestUpdateDataPlaneStatus(t *testing.T) {
10621036
g.Expect(connTracker.SetInstanceIDCallCount()).To(Equal(1))
10631037

10641038
key, id := connTracker.SetInstanceIDArgsForCall(0)
1065-
g.Expect(key).To(Equal("127.0.0.1"))
1039+
g.Expect(key).To(Equal("1234567"))
10661040
g.Expect(id).To(Equal(test.expID))
10671041
})
10681042
}

‎internal/controller/nginx/agent/file.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func (fs *fileService) GetFile(
6565
return nil, status.Error(codes.InvalidArgument, "invalid request")
6666
}
6767

68-
contents, err := fs.getFileContents(req, gi.IPAddress)
68+
contents, err := fs.getFileContents(req, gi.UUID)
6969
if err != nil {
7070
return nil, err
7171
}
@@ -93,7 +93,7 @@ func (fs *fileService) GetFileStream(
9393
return status.Error(codes.InvalidArgument, "invalid request")
9494
}
9595

96-
contents, err := fs.getFileContents(req, gi.IPAddress)
96+
contents, err := fs.getFileContents(req, gi.UUID)
9797
if err != nil {
9898
return err
9999
}
@@ -192,7 +192,7 @@ func (fs *fileService) UpdateOverview(
192192
return &pb.UpdateOverviewResponse{}, agentgrpc.ErrStatusInvalidConnection
193193
}
194194

195-
conn := fs.connTracker.GetConnection(gi.IPAddress)
195+
conn := fs.connTracker.GetConnection(gi.UUID)
196196
if conn.PodName == "" {
197197
return &pb.UpdateOverviewResponse{}, status.Errorf(codes.NotFound, "connection not found")
198198
}

‎internal/controller/nginx/agent/file_test.go‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ func TestGetFile(t *testing.T) {
6868
fs := newFileService(logr.Discard(), depStore, connTracker)
6969

7070
ctx := grpcContext.NewGrpcContext(t.Context(), grpcContext.GrpcInfo{
71-
IPAddress: "127.0.0.1",
71+
UUID: "1234567",
7272
})
7373

7474
req := &pb.GetFileRequest{
@@ -121,7 +121,7 @@ func TestGetFile_InvalidRequest(t *testing.T) {
121121
fs := newFileService(logr.Discard(), depStore, connTracker)
122122

123123
ctx := grpcContext.NewGrpcContext(t.Context(), grpcContext.GrpcInfo{
124-
IPAddress: "127.0.0.1",
124+
UUID: "1234567",
125125
})
126126

127127
req := &pb.GetFileRequest{
@@ -148,7 +148,7 @@ func TestGetFile_ConnectionNotFound(t *testing.T) {
148148
}
149149

150150
ctx := grpcContext.NewGrpcContext(t.Context(), grpcContext.GrpcInfo{
151-
IPAddress: "127.0.0.1",
151+
UUID: "1234567",
152152
})
153153

154154
resp, err := fs.GetFile(ctx, req)
@@ -181,7 +181,7 @@ func TestGetFile_DeploymentNotFound(t *testing.T) {
181181
}
182182

183183
ctx := grpcContext.NewGrpcContext(t.Context(), grpcContext.GrpcInfo{
184-
IPAddress: "127.0.0.1",
184+
UUID: "1234567",
185185
})
186186

187187
resp, err := fs.GetFile(ctx, req)
@@ -217,7 +217,7 @@ func TestGetFile_FileNotFound(t *testing.T) {
217217
}
218218

219219
ctx := grpcContext.NewGrpcContext(t.Context(), grpcContext.GrpcInfo{
220-
IPAddress: "127.0.0.1",
220+
UUID: "1234567",
221221
})
222222

223223
resp, err := fs.GetFile(ctx, req)
@@ -264,7 +264,7 @@ func TestGetFileStream(t *testing.T) {
264264
fs := newFileService(logr.Discard(), depStore, connTracker)
265265

266266
ctx := grpcContext.NewGrpcContext(t.Context(), grpcContext.GrpcInfo{
267-
IPAddress: "127.0.0.1",
267+
UUID: "1234567",
268268
})
269269

270270
req := &pb.GetFileRequest{
@@ -324,7 +324,7 @@ func TestGetFileStream_InvalidRequest(t *testing.T) {
324324
fs := newFileService(logr.Discard(), depStore, connTracker)
325325

326326
ctx := grpcContext.NewGrpcContext(t.Context(), grpcContext.GrpcInfo{
327-
IPAddress: "127.0.0.1",
327+
UUID: "1234567",
328328
})
329329

330330
// no filemeta
@@ -397,7 +397,7 @@ func TestUpdateOverview(t *testing.T) {
397397
}
398398

399399
ctx := grpcContext.NewGrpcContext(t.Context(), grpcContext.GrpcInfo{
400-
IPAddress: "127.0.0.1",
400+
UUID: "1234567",
401401
})
402402

403403
fs := newFileService(logr.Discard(), depStore, connTracker)
@@ -487,7 +487,7 @@ func TestUpdateOverview_ConnectionNotFound(t *testing.T) {
487487
}
488488

489489
ctx := grpcContext.NewGrpcContext(t.Context(), grpcContext.GrpcInfo{
490-
IPAddress: "127.0.0.1",
490+
UUID: "1234567",
491491
})
492492

493493
resp, err := fs.UpdateOverview(ctx, req)
@@ -526,7 +526,7 @@ func TestUpdateOverview_DeploymentNotFound(t *testing.T) {
526526
}
527527

528528
ctx := grpcContext.NewGrpcContext(t.Context(), grpcContext.GrpcInfo{
529-
IPAddress: "127.0.0.1",
529+
UUID: "1234567",
530530
})
531531

532532
resp, err := fs.UpdateOverview(ctx, req)

‎internal/controller/nginx/agent/grpc/context/context.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ import (
66

77
// GrpcInfo for storing identity information for the gRPC client.
88
type GrpcInfo struct {
9-
Token string `json:"token"` // auth token that was provided by the gRPC client
10-
IPAddress string `json:"ip_address"` // ip address of the agent
9+
UUID string `json:"uuid"` // unique identifier for the gRPC client
10+
Token string `json:"token"` // auth token that was provided by the gRPC client
1111
}
1212

1313
type contextGRPCKey struct{}

‎internal/controller/nginx/agent/grpc/context/context_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ func TestGrpcInfoInContext(t *testing.T) {
1313
t.Parallel()
1414
g := NewWithT(t)
1515

16-
grpcInfo := grpcContext.GrpcInfo{IPAddress: "192.168.1.1"}
16+
grpcInfo := grpcContext.GrpcInfo{Token: "test"}
1717

1818
newCtx := grpcContext.NewGrpcContext(context.Background(), grpcInfo)
1919
info, ok := grpcContext.GrpcInfoFromContext(newCtx)

0 commit comments

Comments
 (0)