Skip to content
Merged
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
14 changes: 2 additions & 12 deletions internal/cli/cluster.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -59,7 +59,6 @@ func newClusterInfoCmd() *cobra.Command {
kubeconfigPath string
contextOverride string
nsOverride string
ingestorSAName string
tokenExpiry int64
)

Expand DownExpand Up@@ -93,7 +92,6 @@ Exit codes:
cmd.Context(),
printerFor(cmd),
kubeconfigPath, contextOverride, nsOverride,
ingestorSAName,
tokenExpiry,
)
},
Expand All@@ -105,9 +103,6 @@ Exit codes:
"name of the kubeconfig context to use (default: kubeconfig's current-context)")
cmd.Flags().StringVarP(&nsOverride, "namespace", "n", "",
"namespace where your tracebloc client is installed (default: the context's namespace, or 'default')")
cmd.Flags().StringVar(&ingestorSAName, "ingestor-sa", "",
"override the ingestor ServiceAccount name (default: \"ingestor\", the chart default; "+
"set this if you customized `ingestionAuthz.serviceAccountName` in your client's install)")
cmd.Flags().Int64Var(&tokenExpiry, "token-expiry-seconds", 600,
"requested SA token expiration in seconds (default 600 = 10 min; ignored for static-secret fallback)")

Expand All@@ -118,7 +113,6 @@ func runClusterInfo(
ctx context.Context,
p *ui.Printer,
kubeconfigPath, contextOverride, nsOverride string,
ingestorSAOverride string,
tokenExpiry int64,
) error {
p.Banner("tracebloc", "cluster diagnostics")
Expand DownExpand Up@@ -171,12 +165,8 @@ func runClusterInfo(
// `data ingest` will target, so it must not show the pre-scan default.
p.Field("namespace", resolved.Namespace)

// Apply the SA-name override here. Discovery doesn't read the
// name from the cluster (see #7); customers with a non-default
// name pass --ingestor-sa.
if ingestorSAOverride != "" {
release.IngestorSAName = ingestorSAOverride
}
// release.IngestorSAName is discovered from the ingestionAuthz ConfigMap by
// DiscoverParentRelease (#7) — no --ingestor-sa override needed.

p.Section("Client install")
p.Field("name", release.ReleaseName)
Expand Down
2 changes: 1 addition & 1 deletion internal/cli/coverage_test.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -226,7 +226,7 @@ func TestRunClusterInfo_BadKubeconfigExitsThree(t *testing.T) {
}

var buf bytes.Buffer
err := runClusterInfo(context.Background(), ui.New(&buf), bad, "", "", "", 600)
err := runClusterInfo(context.Background(), ui.New(&buf), bad, "", "", 600)
if err == nil {
t.Fatal("runClusterInfo with a broken kubeconfig returned nil; want an exitError")
}
Expand Down
17 changes: 3 additions & 14 deletions internal/cli/data.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -103,12 +103,6 @@ func newDataIngestCmd() *cobra.Command {
noInput bool
outputJSON bool

// Ingestor SA name override. Used as the ServiceAccountName
// of the ephemeral stage Pod, so the Pod inherits whatever
// imagePullSecrets + PSA exemptions the admin already
// configured for that SA.
ingestorSAName string

// Stage Pod image override. Defaults to the digest-pinned
// alpine that ships with the CLI; air-gapped customers
// override this to an image their registry mirror serves.
Expand DownExpand Up@@ -210,7 +204,6 @@ Exit codes:
SchemaFlag: schemaFlag,
DryRun: dryRun,
Overwrite: overwrite,
IngestorSAName: ingestorSAName,
StagePodImage: stagePodImage,
Detach: detach,
IdempotencyKey: idempotencyKey,
Expand DownExpand Up@@ -267,9 +260,6 @@ Exit codes:
"disable interactive prompts; fail on missing required values (for CI/scripts)")
cmd.Flags().BoolVar(&outputJSON, "output-json", false,
"emit a machine-readable JSON result on stdout (human output → stderr; implies --no-input)")
cmd.Flags().StringVar(&ingestorSAName, "ingestor-sa", "",
"override the ingestor ServiceAccount name (default: \"ingestor\"); "+
"set this if you customized ingestionAuthz.serviceAccountName in your client's install")
cmd.Flags().StringVar(&stagePodImage, "stage-pod-image", "",
"override the ephemeral stage Pod's image (default: digest-pinned alpine 3.20 baked into the CLI). "+
"Pin by digest in your override too — tag-only refs drift silently.")
Expand DownExpand Up@@ -302,7 +292,6 @@ type runDataIngestArgs struct {
SchemaFlag string // raw --schema; resolved or inferred after Discover (tabular)
DryRun bool
Overwrite bool
IngestorSAName string
StagePodImage string

// Printer renders the pre-flight summary + status output. Built in
Expand DownExpand Up@@ -652,9 +641,9 @@ other collaborators train against it without ever seeing the raw files.`))
return binding.explain(err)
}
resolved, cs, release, pvc := target.Resolved, target.Clientset, target.Release, target.PVC
if a.IngestorSAName != "" {
release.IngestorSAName = a.IngestorSAName
}
// release.IngestorSAName is discovered from the ingestionAuthz ConfigMap by
// DiscoverParentRelease (#7) and flows into the stage/teardown pods + the
// jobs-manager token mint below — no --ingestor-sa override.

// 7. Show what we found on the cluster — the customer's last look
// before any bytes move.
Expand Down
90 changes: 72 additions & 18 deletions internal/cluster/discover.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,6 +10,8 @@ import (
appsv1 "k8s.io/api/apps/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"

"gopkg.in/yaml.v3"
)

// ErrNoParentRelease is the sentinel for DiscoverParentRelease's "the namespace
Expand DownExpand Up@@ -57,14 +59,12 @@ type ParentRelease struct {
JobsManagerServiceName string
JobsManagerPort int

// IngestorSAName is the name of the ServiceAccount the chart's
// hook pods run as. Today this is always the chart's default
// "ingestor". Customers who set `ingestionAuthz.serviceAccountName`
// to a non-default name in the parent client chart need to
// override via `tracebloc cluster info --ingestor-sa=<name>`
// (and similarly for the future `dataset push` command). Reading
// the name from the cluster's ingestionAuthz ConfigMap so this
// flag becomes unnecessary is a v0.2 follow-up (see #7).
// IngestorSAName is the name of the ServiceAccount the CLI mints a
// jobs-manager token for and runs its stage/teardown pods as. It is
// discovered from the chart's ingestionAuthz ConfigMap — the policy
// jobs-manager actually enforces (see discoverIngestorSAName, #7) — and
// defaults to the chart default "ingestor" when the ConfigMap is absent
// (older chart), unreadable, or ambiguous.
IngestorSAName string

// IngestorImageDigest is the canonical digest the cluster's
Expand DownExpand Up@@ -171,17 +171,16 @@ func DiscoverParentRelease(ctx context.Context, cs kubernetes.Interface, namespa
release.JobsManagerServiceName = svc
release.JobsManagerPort = jobsManagerPort

// Read INGESTOR_IMAGE_DIGEST from jobs-manager's pod-spec env.
// The chart pipes images.ingestor.digest through to here.
//
// SA name is NOT discovered today — the chart doesn't surface
// `ingestionAuthz.serviceAccountName` through the jobs-manager
// env, and reading the ingestionAuthz ConfigMap to learn it is a
// v0.2 follow-up (see #7). We default to "ingestor" (the chart
// default); customers who renamed it pass --ingestor-sa from
// the CLI. Bugbot caught the earlier version that incorrectly
// claimed to read the SA name from env.
// The ingestor ServiceAccount name. Default to the chart default
// ("ingestor"), then let the ingestionAuthz ConfigMap override it — that
// ConfigMap IS the policy jobs-manager enforces, so it is the authoritative
// source for which SA may call submit-ingestion-run (#7). Best-effort:
// a missing ConfigMap (older chart), an RBAC denial, or an ambiguous
// policy all leave the default in place.
release.IngestorSAName = "ingestor"
if sa := discoverIngestorSAName(ctx, cs, namespace, release.ReleaseName); sa != "" {
release.IngestorSAName = sa
}
if len(d.Spec.Template.Spec.Containers) > 0 {
for _, env := range d.Spec.Template.Spec.Containers[0].Env {
if env.Name == "INGESTOR_IMAGE_DIGEST" {
Expand All@@ -193,6 +192,61 @@ func DiscoverParentRelease(ctx context.Context, cs kubernetes.Interface, namespa
return release, nil
}

// discoverIngestorSAName reads the chart's ingestion-authz ConfigMap
// (`<release>-ingestion-authz`, key `ingestion-authz.yaml`) and returns the
// ServiceAccount the CLI should mint a token for — the one jobs-manager will
// accept on POST /internal/submit-ingestion-run. It replaces the old hardcoded
// "ingestor" default + the manual --ingestor-sa flag (#7).
//
// The policy lists `allowed[]` entries of (service_account, namespace,
// table_prefixes). We keep only entries whose namespace matches the namespace
// we'll mint in (the rendered ConfigMap always populates namespace, defaulting
// to the release namespace), then require exactly one distinct non-empty
// service_account. Anything else — ConfigMap absent (older chart), RBAC denial,
// empty policy, or two different SAs — returns "" so the caller keeps the
// "ingestor" default rather than guessing an SA the server would reject.
// Best-effort by contract: never returns an error.
func discoverIngestorSAName(ctx context.Context, cs kubernetes.Interface, namespace, releaseName string) string {
if releaseName == "" {
return ""
}
cm, err := cs.CoreV1().ConfigMaps(namespace).Get(
ctx, releaseName+"-ingestion-authz", metav1.GetOptions{})
if err != nil {
// NotFound (older chart / no ingestor subchart) or an RBAC/API error —
// both non-fatal; fall back to the default.
return ""
}
raw, ok := cm.Data["ingestion-authz.yaml"]
if !ok {
return ""
}
var policy struct {
Allowed []struct {
ServiceAccount string `yaml:"service_account"`
Namespace string `yaml:"namespace"`
} `yaml:"allowed"`
}
if err := yaml.Unmarshal([]byte(raw), &policy); err != nil {
return ""
}
sa := ""
for _, e := range policy.Allowed {
// Only entries that apply to the namespace we mint in — a token minted
// here is worthless against a policy entry scoped to a different ns.
if e.Namespace != namespace || e.ServiceAccount == "" {
continue
}
switch {
case sa == "":
sa = e.ServiceAccount
case sa != e.ServiceAccount:
return "" // ambiguous — more than one SA for this namespace; keep the default
}
}
return sa
}

// FindClientNamespaces scans every namespace the kubeconfig user may list for
// jobs-manager Deployments (the same selector + name filter DiscoverParentRelease
// uses) and returns the sorted, de-duplicated namespaces hosting one. It backs
Expand Down
75 changes: 75 additions & 0 deletions internal/cluster/discover_test.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -330,3 +330,78 @@ func siblingDeployment(name, namespace string) *appsv1.Deployment {
},
}
}

// ingestionAuthzCM builds the chart's `<release>-ingestion-authz` ConfigMap
// with the given rendered policy body under the `ingestion-authz.yaml` key —
// the contract DiscoverParentRelease reads the ingestor SA name from (#7).
func ingestionAuthzCM(release, namespace, policyYAML string) *corev1.ConfigMap {
return &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: release + "-ingestion-authz",
Namespace: namespace,
},
Data: map[string]string{"ingestion-authz.yaml": policyYAML},
}
}

// A renamed ingestor SA in the ingestionAuthz policy is discovered and
// overrides the "ingestor" default — the whole point of #7 (no --ingestor-sa).
func TestDiscoverParentRelease_DiscoversRenamedSAFromConfigMap(t *testing.T) {
const ns = "tracebloc"
cs := fake.NewClientset(
jobsManagerDeployment("tracebloc", ns, "client-1.3.5", "1.3.5", "d"),
jobsManagerService("jobs-manager", ns),
ingestionAuthzCM("tracebloc", ns,
"allowed:\n - service_account: \"foo-ingestor\"\n namespace: \""+ns+"\"\n table_prefixes: [\"\"]\n"),
)
release, err := DiscoverParentRelease(context.Background(), cs, ns)
if err != nil {
t.Fatalf("DiscoverParentRelease: %v", err)
}
if release.IngestorSAName != "foo-ingestor" {
t.Errorf("IngestorSAName = %q, want %q (discovered from the ingestionAuthz ConfigMap)",
release.IngestorSAName, "foo-ingestor")
}
}

// Two entries naming different SAs for the same namespace is ambiguous — keep
// the "ingestor" default rather than guess one the server would reject.
func TestDiscoverParentRelease_AmbiguousSAKeepsDefault(t *testing.T) {
const ns = "tracebloc"
cs := fake.NewClientset(
jobsManagerDeployment("tracebloc", ns, "client-1.3.5", "1.3.5", "d"),
jobsManagerService("jobs-manager", ns),
ingestionAuthzCM("tracebloc", ns,
"allowed:\n - service_account: \"sa-one\"\n namespace: \""+ns+"\"\n"+
" - service_account: \"sa-two\"\n namespace: \""+ns+"\"\n"),
)
release, err := DiscoverParentRelease(context.Background(), cs, ns)
if err != nil {
t.Fatalf("DiscoverParentRelease: %v", err)
}
if release.IngestorSAName != "ingestor" {
t.Errorf("IngestorSAName = %q, want the default %q on an ambiguous policy",
release.IngestorSAName, "ingestor")
}
}

// An entry scoped to a DIFFERENT namespace than the one we resolve/mint in must
// be ignored — a token minted in our namespace is worthless against it. Pins
// the namespace-gate (#7 stress-review refinement).
func TestDiscoverParentRelease_CrossNamespaceSAIgnored(t *testing.T) {
const ns = "tracebloc"
cs := fake.NewClientset(
jobsManagerDeployment("tracebloc", ns, "client-1.3.5", "1.3.5", "d"),
jobsManagerService("jobs-manager", ns),
ingestionAuthzCM("tracebloc", ns,
"allowed:\n - service_account: \"other-ns-sa\"\n namespace: \"somewhere-else\"\n table_prefixes: [\"\"]\n"),
)
release, err := DiscoverParentRelease(context.Background(), cs, ns)
if err != nil {
t.Fatalf("DiscoverParentRelease: %v", err)
}
if release.IngestorSAName != "ingestor" {
t.Errorf("IngestorSAName = %q, want the default %q (cross-namespace entry must be ignored)",
release.IngestorSAName, "ingestor")
}
}
6 changes: 3 additions & 3 deletions internal/push/pod.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -123,9 +123,9 @@ type PodSpecOptions struct {
// Image overrides DefaultStagePodImage. Empty = use default.
Image string

// ServiceAccountName is the SA the Pod runs as. Phase 2's
// discovery surfaces this as `ingestor` (the chart default) or
// whatever the customer's `--ingestor-sa` flag overrides to.
// ServiceAccountName is the SA the Pod runs as. Discovery surfaces
// this from the ingestionAuthz ConfigMap (the chart default `ingestor`
// when the policy is absent/ambiguous — see cluster.discoverIngestorSAName).
// Using the chart's existing SA means the Pod inherits any
// imagePullSecrets and PSA exemptions the admin already
// configured for it.
Expand Down
4 changes: 2 additions & 2 deletions internal/push/stage.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -25,8 +25,8 @@ type StageOptions struct {
// Tests: a fakeExecutor (see stream_test.go).
Executor Executor

// Namespace + IngestorSAName come from Phase 2's parent-release
// discovery (with optional --ingestor-sa override).
// Namespace + IngestorSAName come from parent-release discovery
// (IngestorSAName is read from the ingestionAuthz ConfigMap, #7).
Namespace string
IngestorSAName string

Expand Down
13 changes: 6 additions & 7 deletions internal/push/stage_test.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -188,13 +188,12 @@ func TestStage_OrphanWarningSurfaces(t *testing.T) {
}
}

// TestStage_IngestorSANameFlowsToPod: the --ingestor-sa override
// MUST land on the stage Pod's ServiceAccountName — otherwise the
// flag is silently ignored and customers who renamed the chart's
// ingestor SA get pods running as the default SA (no PVC write
// access). Pin the integration here at the Stage layer since the
// flag wiring's effect is only observable end-to-end. Bugbot
// flagged the missing test coverage on PR-a.
// TestStage_IngestorSANameFlowsToPod: the discovered ingestor SA name
// MUST land on the stage Pod's ServiceAccountName — otherwise customers
// whose ingestionAuthz policy names a non-default SA (#7) get pods
// running as the wrong SA (no PVC write access). Pin the integration
// here at the Stage layer since the wiring's effect is only observable
// end-to-end. Bugbot flagged the missing test coverage on PR-a.
func TestStage_IngestorSANameFlowsToPod(t *testing.T) {
root := imgcDir(t)
layout, err := Discover(root)
Expand Down
Loading