Skip to content

Commit 03f370f

Browse files
committed
review: NamespaceFromPodEnv helper, APIServiceName const, ateclient hardcodes
Three follow-ups from the self-review: - Extract the POD_NAMESPACE-with-SystemNamespace-fallback pattern into installdefaults.NamespaceFromPodEnv() so ateapi and atenet share a single implementation (also makes a third call site one line instead of four if anyone needs one). - Add installdefaults.PodNamespaceEnv ("POD_NAMESPACE") and APIServiceName ("api") so the constant set covers every name in the canonical install layout that's referenced by Go code. - Route internal/ateclient/builder.go's previously-hardcoded "ate-system" and "api" lookups through installdefaults, so kubectl-ate's port-forward no longer bypasses the new single source of truth. ate-controller (ServiceAccount), ate-api-server-deployment (Deployment), and "api.ate-system.svc" (JWT audience) are still hardcoded but their configurability needs a real flag/discovery story and is out of scope for this PR.
1 parent 9603e49 commit 03f370f

4 files changed

Lines changed: 33 additions & 22 deletions

File tree

cmd/ateapi/main.go

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -136,13 +136,9 @@ func main() {
136136
workerPoolLister := ateFactory.Api().V1alpha1().WorkerPools().Lister()
137137
sandboxConfigLister := ateFactory.Api().V1alpha1().SandboxConfigs().Lister()
138138

139-
// atelet shares ateapi's namespace in every supported deployment topology.
140-
// POD_NAMESPACE comes from Kubernetes' downward API; the install fallback
141-
// keeps non-k8s invocations (tests, local dev) working.
142-
ateletNamespace := os.Getenv("POD_NAMESPACE")
143-
if ateletNamespace == "" {
144-
ateletNamespace = installdefaults.SystemNamespace
145-
}
139+
// atelet shares ateapi's namespace in every supported deployment topology,
140+
// so we read it from Kubernetes' downward API rather than expose a flag.
141+
ateletNamespace := installdefaults.NamespaceFromPodEnv()
146142
slog.InfoContext(ctx, "Resolved atelet namespace", slog.String("atelet-namespace", ateletNamespace))
147143

148144
workerPodInformerFactory, workerPodInformer := controlapi.WorkerPodInformer(clientset)

cmd/atenet/internal/dns.go

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -89,14 +89,10 @@ func NewDnsCmd() *cobra.Command {
8989
return fmt.Errorf("failed to initialize cluster client: %w", err)
9090
}
9191

92-
// atenet shares the system namespace with atenet-router and
93-
// substrate's CoreDNS in every supported deployment topology.
94-
// POD_NAMESPACE comes from Kubernetes' downward API; the install
95-
// fallback keeps non-k8s invocations (tests, local dev) working.
96-
systemNamespace := os.Getenv("POD_NAMESPACE")
97-
if systemNamespace == "" {
98-
systemNamespace = installdefaults.SystemNamespace
99-
}
92+
// atenet shares its namespace with atenet-router and substrate's
93+
// CoreDNS in every supported deployment topology, so we read it
94+
// from Kubernetes' downward API rather than expose a flag.
95+
systemNamespace := installdefaults.NamespaceFromPodEnv()
10096
slog.InfoContext(ctx, "Resolved system namespace", slog.String("system-namespace", systemNamespace))
10197

10298
dnsController := &dns.Controller{

internal/ateclient/builder.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"strings"
2525
"sync"
2626

27+
"github.com/agent-substrate/substrate/internal/installdefaults"
2728
"github.com/agent-substrate/substrate/pkg/proto/ateapipb"
2829
"go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc"
2930
"go.opentelemetry.io/otel"
@@ -132,22 +133,22 @@ func dialPortForward(ctx context.Context, kubeconfigPath, k8sContext string, tra
132133
return nil, fmt.Errorf("failed to create k8s client: %w", err)
133134
}
134135

135-
// Look up the 'api' Service to dynamically get its pod selector
136-
svc, err := clientset.CoreV1().Services("ate-system").Get(ctx, "api", metav1.GetOptions{})
136+
// Look up the ateapi Service to dynamically get its pod selector.
137+
svc, err := clientset.CoreV1().Services(installdefaults.SystemNamespace).Get(ctx, installdefaults.APIServiceName, metav1.GetOptions{})
137138
if err != nil {
138-
return nil, fmt.Errorf("failed to get api service: %w", err)
139+
return nil, fmt.Errorf("failed to get ateapi service %s/%s: %w", installdefaults.SystemNamespace, installdefaults.APIServiceName, err)
139140
}
140141
selector := labels.SelectorFromSet(svc.Spec.Selector).String()
141142

142143
// Find the pods backing the service
143-
pods, err := clientset.CoreV1().Pods("ate-system").List(ctx, metav1.ListOptions{
144+
pods, err := clientset.CoreV1().Pods(installdefaults.SystemNamespace).List(ctx, metav1.ListOptions{
144145
LabelSelector: selector,
145146
})
146147
if err != nil {
147148
return nil, fmt.Errorf("failed to list ateapi pods: %w", err)
148149
}
149150
if len(pods.Items) == 0 {
150-
return nil, fmt.Errorf("no ate-api-server pods found in ate-system namespace")
151+
return nil, fmt.Errorf("no ate-api-server pods found in %q namespace", installdefaults.SystemNamespace)
151152
}
152153
targetPod := pods.Items[0]
153154

@@ -254,7 +255,7 @@ func jwtDialOptions(ctx context.Context, clientset *kubernetes.Clientset) ([]grp
254255
ExpirationSeconds: &expirationSeconds,
255256
},
256257
}
257-
token, err := clientset.CoreV1().ServiceAccounts("ate-system").CreateToken(ctx, "ate-client", tokenRequest, metav1.CreateOptions{})
258+
token, err := clientset.CoreV1().ServiceAccounts(installdefaults.SystemNamespace).CreateToken(ctx, "ate-client", tokenRequest, metav1.CreateOptions{})
258259
if err != nil {
259260
return nil, fmt.Errorf("failed to request ateapi bearer token: %w", err)
260261
}
@@ -267,7 +268,7 @@ func jwtDialOptions(ctx context.Context, clientset *kubernetes.Clientset) ([]grp
267268
func isJWTMode(ctx context.Context, clientset *kubernetes.Clientset) (bool, error) {
268269
// TODO: Replace deployment introspection with an explicit client-readable
269270
// config file once ateapi auth mode is part of install/runtime config.
270-
deployment, err := clientset.AppsV1().Deployments("ate-system").Get(ctx, "ate-api-server-deployment", metav1.GetOptions{})
271+
deployment, err := clientset.AppsV1().Deployments(installdefaults.SystemNamespace).Get(ctx, "ate-api-server-deployment", metav1.GetOptions{})
271272
if err != nil {
272273
return false, fmt.Errorf("failed to get ate-api-server deployment: %w", err)
273274
}

internal/installdefaults/installdefaults.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,30 @@
1818
// the canonical layout pass actual values via the corresponding flags.
1919
package installdefaults
2020

21+
import "os"
22+
2123
const (
2224
// SystemNamespace is the namespace where substrate's control-plane
2325
// components and the atelet DaemonSet run.
2426
SystemNamespace = "ate-system"
27+
// APIServiceName is the Service name of ate-api-server.
28+
APIServiceName = "api"
2529
// RouterServiceName is the Service name of atenet-router.
2630
RouterServiceName = "atenet-router"
2731
// DNSServiceName is the Service name of substrate's CoreDNS.
2832
DNSServiceName = "dns"
33+
34+
// PodNamespaceEnv is the conventional env var name for the namespace
35+
// a pod is running in, exposed via Kubernetes' downward API.
36+
PodNamespaceEnv = "POD_NAMESPACE"
2937
)
38+
39+
// NamespaceFromPodEnv returns the namespace from the PodNamespaceEnv env
40+
// var when set (typically populated via Kubernetes' downward API), and
41+
// falls back to SystemNamespace for non-k8s invocations (tests, local dev).
42+
func NamespaceFromPodEnv() string {
43+
if ns := os.Getenv(PodNamespaceEnv); ns != "" {
44+
return ns
45+
}
46+
return SystemNamespace
47+
}

0 commit comments

Comments
 (0)