diff --git a/go.mod b/go.mod index deca33e7cd..66a5aff49d 100644 --- a/go.mod +++ b/go.mod @@ -69,6 +69,7 @@ require ( golang.org/x/sys v0.48.0 golang.org/x/time v0.15.0 google.golang.org/api v0.274.0 + google.golang.org/genproto v0.0.0-20260319201613-d00831a3d3e7 google.golang.org/genproto/googleapis/rpc v0.0.0-20260803160001-6ac0973c030d google.golang.org/grpc v1.83.2 google.golang.org/protobuf v1.36.12 @@ -250,7 +251,6 @@ require ( golang.org/x/text v0.42.0 // indirect gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect gonum.org/v1/gonum v0.17.0 // indirect - google.golang.org/genproto v0.0.0-20260319201613-d00831a3d3e7 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20260803160001-6ac0973c030d // indirect gopkg.in/evanphx/json-patch.v4 v4.13.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect diff --git a/hack/microvm-assets/stage-to-gcs.sh b/hack/microvm-assets/stage-to-gcs.sh index ba1afad6bd..91703fe248 100755 --- a/hack/microvm-assets/stage-to-gcs.sh +++ b/hack/microvm-assets/stage-to-gcs.sh @@ -18,8 +18,9 @@ # kata-assets/, where atelet fetches it through the node's object-store plugin # (per manifests/microvm/sandboxconfig-microvm.yaml.tmpl). The plugin refuses # assets stored under a snapshot location (/atespaces/), -# so keep them outside it. The GKE counterpart of stage-to-rustfs.sh. Run after -# assemble.sh has produced $OUT. +# so keep them outside it. tools/setup-gcp grants nodes read access to +# kata-assets/ only, so the prefix must not change. The GKE counterpart of +# stage-to-rustfs.sh. Run after assemble.sh has produced $OUT. # # Requires the `gcloud` CLI authenticated for the bucket's project. Env: OUT (asset # dir, default ./bin/microvm-assets/amd64), BUCKET (default ate-snapshots), diff --git a/tools/setup-gcp/README.md b/tools/setup-gcp/README.md index 35cac2714d..116d962f34 100644 --- a/tools/setup-gcp/README.md +++ b/tools/setup-gcp/README.md @@ -217,16 +217,16 @@ go run ./tools/setup-gcp create iam [flags] | `--bucket` | GCS bucket name. | `BUCKET_NAME` | None (Required for bucket bindings*) | | `--gke-nodes` | Grant GKE nodes permission to pull images. | - | `true` | | `--atelet` | Grant atelet project-level permissions. | - | `true` | -| `--bucket-bindings` | Grant atelet and ate-api-server access to the snapshot bucket. | - | `true` | +| `--bucket-bindings` | Grant ate-api-server access to the snapshot bucket, and atelet read access to the micro-VM assets in it. | - | `true` | *\*Note: Required for bucket bindings unless the `BUCKET_NAME` environment variable is set.* #### What `create iam` actually grants On a fresh GCP project these bindings do not exist, and nothing else creates -them — without them atelet cannot read or write external snapshots (`403` from -GCS on the first suspend/resume), ate-api cannot copy or collect them, and -nodes cannot pull images. `bootstrap` and `create iam` apply them for you; the +them — without them atelet cannot fetch micro-VM assets, ate-api cannot copy, +collect, or issue per-actor tokens for snapshots (`403` from GCS on the first +suspend/resume), and nodes cannot pull images. `bootstrap` and `create iam` apply them for you; the table below is the reference for auditing them, or for applying them by hand in projects where you cannot run the tool with project-level IAM permissions. @@ -241,10 +241,8 @@ cluster must have it enabled too, or their GCS calls fail with `401`). | Member | Role | Resource | Why | | :--- | :--- | :--- | :--- | -| atelet WI principal¹ | `roles/storage.objectAdmin` | project² | Read/write actor snapshots in GCS | | atelet WI principal¹ | `roles/artifactregistry.reader` | project² | Pull sandbox runtime assets | -| atelet WI principal¹ | `roles/storage.objectAdmin` | snapshot bucket | Read/write snapshot objects | -| atelet WI principal¹ | `roles/storage.bucketViewer` | snapshot bucket | List/stat the snapshot bucket | +| atelet WI principal¹ | `roles/storage.objectViewer` | snapshot bucket, objects under `kata-assets/` only | Read micro-VM sandbox assets. Snapshot transfers run on per-actor tokens from the credential exchange | | ate-api-server WI principal¹ | `roles/storage.objectAdmin` | snapshot bucket | Copy external snapshots for tags, delete the ones nothing refers to | | ate-api-server WI principal¹ | `roles/storage.bucketViewer` | snapshot bucket | List/stat the snapshot bucket | | default compute SA³ | `roles/storage.objectViewer` | project² | Nodes pull images from GCS-backed registries | @@ -261,16 +259,15 @@ WI="principal://iam.googleapis.com/projects/${PROJECT_NUMBER}/locations/global/w ATELET="${WI}/atelet" ATE_API="${WI}/ate-api-server" -gcloud projects add-iam-policy-binding "${PROJECT_ID}" \ - --member="${ATELET}" --role=roles/storage.objectAdmin gcloud projects add-iam-policy-binding "${PROJECT_ID}" \ --member="${ATELET}" --role=roles/artifactregistry.reader -for MEMBER in "${ATELET}" "${ATE_API}"; do - gcloud storage buckets add-iam-policy-binding "gs://${BUCKET_NAME}" \ - --member="${MEMBER}" --role=roles/storage.objectAdmin - gcloud storage buckets add-iam-policy-binding "gs://${BUCKET_NAME}" \ - --member="${MEMBER}" --role=roles/storage.bucketViewer -done +gcloud storage buckets add-iam-policy-binding "gs://${BUCKET_NAME}" \ + --member="${ATE_API}" --role=roles/storage.objectAdmin +gcloud storage buckets add-iam-policy-binding "gs://${BUCKET_NAME}" \ + --member="${ATE_API}" --role=roles/storage.bucketViewer +gcloud storage buckets add-iam-policy-binding "gs://${BUCKET_NAME}" \ + --member="${ATELET}" --role=roles/storage.objectViewer \ + --condition="title=objects under kata-assets/,expression=resource.name.startsWith(\"projects/_/buckets/${BUCKET_NAME}/objects/kata-assets/\")" ``` ### 5. Create Dashboards diff --git a/tools/setup-gcp/cmd/bucket.go b/tools/setup-gcp/cmd/bucket.go index 51184f331b..cd154a4950 100644 --- a/tools/setup-gcp/cmd/bucket.go +++ b/tools/setup-gcp/cmd/bucket.go @@ -24,8 +24,10 @@ import ( "strings" "cloud.google.com/go/iam" + "cloud.google.com/go/iam/apiv1/iampb" "cloud.google.com/go/storage" "github.com/spf13/cobra" + "google.golang.org/genproto/googleapis/type/expr" ) func createSnapshotBucket(ctx context.Context, cfg *Config) error { @@ -92,15 +94,39 @@ func createSnapshotBucket(ctx context.Context, cfg *Config) error { return nil } -// snapshotBucketRoles are the roles every Substrate component that touches the -// snapshot bucket needs: objectAdmin to read, write and delete the objects a -// snapshot is made of, and bucketViewer to resolve the bucket itself. -var snapshotBucketRoles = []string{"roles/storage.objectAdmin", "roles/storage.bucketViewer"} +// assetPrefix is where hack/install-microvm-deps.sh stages micro-VM sandbox +// assets in the snapshot bucket. It sits outside every snapshot location. +const assetPrefix = "kata-assets/" -// snapshotBucketSubjects are the Kubernetes service accounts granted those -// roles. atelet writes and reads external snapshots; ate-api-server copies -// them for tags and deletes the ones nothing refers to any more. -var snapshotBucketSubjects = []string{"atelet", "ate-api-server"} +// bucketGrant is a role on the snapshot bucket for an ate-system Kubernetes +// service account, limited to objects under prefix when prefix is set. +type bucketGrant struct { + subject string + role string + prefix string +} + +// snapshotBucketGrants are the roles setup-gcp grants on the snapshot bucket. +// ate-api-server copies snapshots for tags and deletes the ones nothing refers +// to. atelet's object-store plugin reads only micro-VM sandbox assets with its +// own identity: on GCS every snapshot transfer runs on a token scoped to its +// actor, issued by the credential exchange. +var snapshotBucketGrants = []bucketGrant{ + {subject: "ate-api-server", role: "roles/storage.objectAdmin"}, + {subject: "ate-api-server", role: "roles/storage.bucketViewer"}, + {subject: "atelet", role: "roles/storage.objectViewer", prefix: assetPrefix}, +} + +// condition limits g to objects under its prefix in bucket, or is nil. +func (g bucketGrant) condition(bucket string) *expr.Expr { + if g.prefix == "" { + return nil + } + return &expr.Expr{ + Title: "objects under " + g.prefix, + Expression: fmt.Sprintf(`resource.name.startsWith("projects/_/buckets/%s/objects/%s")`, bucket, g.prefix), + } +} func createIamPolicyBindings(ctx context.Context, cfg *Config) error { client, err := storage.NewClient(ctx) @@ -109,44 +135,52 @@ func createIamPolicyBindings(ctx context.Context, cfg *Config) error { } defer client.Close() - bucket := client.Bucket(cfg.BucketName) - policy, err := bucket.IAM().Policy(ctx) + // Version 3, since a policy holding a conditional binding cannot be read + // at version 1. + handle := client.Bucket(cfg.BucketName).IAM().V3() + policy, err := handle.Policy(ctx) if err != nil { return fmt.Errorf("get bucket iam policy: %w", err) } - - changed := false - for _, subject := range snapshotBucketSubjects { - member := fmt.Sprintf("principal://iam.googleapis.com/projects/%s/locations/global/workloadIdentityPools/%s.svc.id.goog/subject/ns/ate-system/sa/%s", cfg.ProjectNumber, cfg.ProjectID, subject) - for _, role := range snapshotBucketRoles { - if hasUnconditionalBinding(policy, role, member) { - continue - } - slog.Info("Adding role to member", slog.String("bucket", cfg.BucketName), slog.String("role", role), slog.String("member", member)) - policy.Add(member, iam.RoleName(role)) - changed = true - } - } + changed := addBucketGrants(policy, cfg, snapshotBucketGrants) if !changed { slog.Info("IAM policy is already correct", slog.String("bucket", cfg.BucketName)) return nil } slog.Info("Setting IAM policy for bucket", slog.String("bucket", cfg.BucketName)) - err = bucket.IAM().SetPolicy(ctx, policy) - if err != nil { + if err := handle.SetPolicy(ctx, policy); err != nil { return fmt.Errorf("set bucket iam policy: %w", err) } - return nil } -// hasUnconditionalBinding reports whether the policy already grants role to -// member without a condition. Conditional bindings are ignored: they may not -// apply to the objects Substrate touches. -func hasUnconditionalBinding(policy *iam.Policy, role, member string) bool { - for _, b := range policy.InternalProto.Bindings { - if b.Condition == nil && b.Role == role && slices.Contains(b.Members, member) { +// addBucketGrants adds each grant the policy lacks and reports whether it +// added any. +func addBucketGrants(policy *iam.Policy3, cfg *Config, grants []bucketGrant) bool { + changed := false + for _, g := range grants { + member := fmt.Sprintf("principal://iam.googleapis.com/projects/%s/locations/global/workloadIdentityPools/%s.svc.id.goog/subject/ns/ate-system/sa/%s", cfg.ProjectNumber, cfg.ProjectID, g.subject) + cond := g.condition(cfg.BucketName) + if hasBinding(policy.Bindings, g.role, member, cond) { + continue + } + slog.Info("Adding role to member", slog.String("bucket", cfg.BucketName), slog.String("role", g.role), slog.String("member", member), slog.String("condition", cond.GetExpression())) + policy.Bindings = append(policy.Bindings, &iampb.Binding{Role: g.role, Members: []string{member}, Condition: cond}) + changed = true + } + return changed +} + +// hasBinding reports whether bindings already grant role to member under the +// same condition, or without one when cond is nil. A binding under another +// condition does not count: it may not cover the objects Substrate touches. +func hasBinding(bindings []*iampb.Binding, role, member string, cond *expr.Expr) bool { + for _, b := range bindings { + if b.Role != role || !slices.Contains(b.Members, member) { + continue + } + if b.Condition.GetExpression() == cond.GetExpression() { return true } } diff --git a/tools/setup-gcp/cmd/bucket_test.go b/tools/setup-gcp/cmd/bucket_test.go index 5574118199..39921252d7 100644 --- a/tools/setup-gcp/cmd/bucket_test.go +++ b/tools/setup-gcp/cmd/bucket_test.go @@ -16,9 +16,14 @@ package cmd import ( "context" + "slices" + "strings" "testing" + "cloud.google.com/go/iam" + "cloud.google.com/go/iam/apiv1/iampb" "cloud.google.com/go/resourcemanager/apiv3/resourcemanagerpb" + "google.golang.org/genproto/googleapis/type/expr" ) func TestValidateBucketFlags(t *testing.T) { @@ -84,3 +89,75 @@ func TestValidateBucketFlags(t *testing.T) { }) } } + +func TestAddBucketGrants(t *testing.T) { + cfg := &Config{ProjectID: "proj", ProjectNumber: "123", BucketName: "snaps"} + member := func(sa string) string { + return "principal://iam.googleapis.com/projects/123/locations/global/workloadIdentityPools/proj.svc.id.goog/subject/ns/ate-system/sa/" + sa + } + assets := &expr.Expr{Expression: `resource.name.startsWith("projects/_/buckets/snaps/objects/kata-assets/")`} + + tests := []struct { + name string + existing []*iampb.Binding + want []string // role and member of each binding added, "role member [conditional]" + }{ + { + name: "empty policy gets every grant, atelet only on the asset prefix", + want: []string{ + "roles/storage.objectAdmin " + member("ate-api-server"), + "roles/storage.bucketViewer " + member("ate-api-server"), + "roles/storage.objectViewer " + member("atelet") + " conditional", + }, + }, + { + name: "grants already in place add nothing", + existing: []*iampb.Binding{ + {Role: "roles/storage.objectAdmin", Members: []string{member("ate-api-server")}}, + {Role: "roles/storage.bucketViewer", Members: []string{member("ate-api-server")}}, + {Role: "roles/storage.objectViewer", Members: []string{member("atelet")}, Condition: assets}, + }, + }, + { + name: "an unconditional atelet viewer does not stand in for the prefix grant", + existing: []*iampb.Binding{ + {Role: "roles/storage.objectAdmin", Members: []string{member("ate-api-server")}}, + {Role: "roles/storage.bucketViewer", Members: []string{member("ate-api-server")}}, + {Role: "roles/storage.objectViewer", Members: []string{member("atelet")}}, + }, + want: []string{"roles/storage.objectViewer " + member("atelet") + " conditional"}, + }, + { + name: "a grant under another condition does not count", + existing: []*iampb.Binding{ + {Role: "roles/storage.objectAdmin", Members: []string{member("ate-api-server")}, Condition: assets}, + {Role: "roles/storage.bucketViewer", Members: []string{member("ate-api-server")}}, + {Role: "roles/storage.objectViewer", Members: []string{member("atelet")}, Condition: assets}, + }, + want: []string{"roles/storage.objectAdmin " + member("ate-api-server")}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + policy := &iam.Policy3{Bindings: slices.Clone(tc.existing)} + changed := addBucketGrants(policy, cfg, snapshotBucketGrants) + var got []string + for _, b := range policy.Bindings[len(tc.existing):] { + s := b.Role + " " + strings.Join(b.Members, ",") + if b.Condition != nil { + if b.Condition.Expression != assets.Expression { + t.Errorf("condition = %q, want %q", b.Condition.Expression, assets.Expression) + } + s += " conditional" + } + got = append(got, s) + } + if !slices.Equal(got, tc.want) { + t.Errorf("added bindings:\n got %q\n want %q", got, tc.want) + } + if changed != (len(tc.want) > 0) { + t.Errorf("changed = %v, want %v", changed, len(tc.want) > 0) + } + }) + } +} diff --git a/tools/setup-gcp/cmd/iam.go b/tools/setup-gcp/cmd/iam.go index 44af28496d..8bf360bb86 100644 --- a/tools/setup-gcp/cmd/iam.go +++ b/tools/setup-gcp/cmd/iam.go @@ -108,13 +108,13 @@ func grantAteletPermissions(ctx context.Context, cfg *Config) error { member := fmt.Sprintf("principal://iam.googleapis.com/projects/%s/locations/global/workloadIdentityPools/%s.svc.id.goog/subject/ns/ate-system/sa/atelet", cfg.ProjectNumber, cfg.ProjectID) - // TODO: This shouldn't be a project-level binding. We should grant atelet - // access only to the specific bucket and artifact registry repository in - // use. - changed1 := addProjectIamBinding(policy, "roles/storage.objectAdmin", member) - changed2 := addProjectIamBinding(policy, "roles/artifactregistry.reader", member) + // atelet gets no storage role here. On GCS every snapshot transfer runs on + // a token scoped to its actor, and the bucket bindings grant what it reads + // with its own identity, the micro-VM assets. + // TODO: grant artifactregistry.reader on the repository in use only. + changed := addProjectIamBinding(policy, "roles/artifactregistry.reader", member) - if !changed1 && !changed2 { + if !changed { slog.Info("IAM policy already has required GKE node permissions. Skipping update.", slog.String("project", cfg.ProjectID)) return nil } @@ -182,5 +182,5 @@ func init() { iamCmd.Flags().StringVar(&cfg.BucketName, "bucket", getEnv("BUCKET_NAME", ""), "GCS bucket name (required for bucket bindings) [env: BUCKET_NAME]") iamCmd.Flags().Bool("gke-nodes", true, "Grant GKE nodes permission to pull images") iamCmd.Flags().Bool("atelet", true, "Grant atelet project-level permissions") - iamCmd.Flags().Bool("bucket-bindings", true, "Grant atelet access to the snapshot bucket") + iamCmd.Flags().Bool("bucket-bindings", true, "Grant ate-api-server access to the snapshot bucket, and atelet read access to the micro-VM assets in it") }