From 4b0430f0df935adce5257f021053b027c82f6bff Mon Sep 17 00:00:00 2001 From: Ben Vincent Date: Fri, 24 Jul 2026 22:48:20 +1000 Subject: [PATCH] Add fine-grained bucket access: paths, actions, conditions, raw The BucketAccess model only offered three coarse levels (read-only/read-write/ full) applied to the whole bucket. Real grants often need to be scoped to a key prefix, limited to a source network or TLS, restricted to specific actions, or expressed as an arbitrary S3 statement. RGW (Reef 18.2+/Squid) honours the S3 bucket-policy features to do all of this; expose them on BucketAccess while keeping the level as the ergonomic default. - add BucketAccess spec fields: paths (key-prefix scoping), actions (action override), conditions (sourceIPs + secureTransportOnly), rawStatements (arbitrary S3 statements with the principal injected) - extend ceph.Grant + BuildBucketPolicy to render prefixed object resources, custom-action statements, S3 condition blocks, and raw statements, keeping output deterministic (sorted, stable sids) - translate the new spec fields into grants in the Bucket controller and fingerprint grants so distinct fine-grained BucketAccess objects no longer collapse on UID+level alone - regenerate deepcopy + CRDs; add config/samples/04-access-fine-grained.yaml - cover paths, action override, conditions, raw statements and determinism in policy_test.go; document the fields in the README Claude-Session: https://claude.ai/code/session_016CEncETbf8cvy1PhsHfFHM --- README.md | 20 ++ api/v1alpha1/bucketaccess_types.go | 66 ++++++ api/v1alpha1/zz_generated.deepcopy.go | 96 ++++++++- .../bases/ceph.unkin.net_bucketaccesses.yaml | 85 ++++++++ .../ceph.unkin.net_objectstoreusers.yaml | 2 +- config/crd/install.yaml | 87 +++++++- config/samples/04-access-fine-grained.yaml | 78 +++++++ internal/ceph/policy.go | 203 ++++++++++++++---- internal/ceph/policy_test.go | 152 +++++++++++++ internal/controller/bucket_controller.go | 40 +++- 10 files changed, 788 insertions(+), 41 deletions(-) create mode 100644 config/samples/04-access-fine-grained.yaml diff --git a/README.md b/README.md index 90e260e..46d628d 100644 --- a/README.md +++ b/README.md @@ -40,6 +40,26 @@ user for that grant and writes its keys into `spec.secretName` (default `-rgw`). If `userRef` names an existing `ObjectStoreUser`, that user's own credential Secret is reused and only the policy is extended. +#### Fine-grained grants + +The level is the ergonomic default; four optional fields on `BucketAccess` +refine it (see `config/samples/04-access-fine-grained.yaml`): + +- `spec.paths` — scope object access to key prefixes; each becomes the resource + `/*`. The bucket-level `ListBucket` still spans the whole + bucket. +- `spec.actions` — grant exactly these S3 actions instead of the level's set (on + the bucket and its, optionally prefixed, objects). +- `spec.conditions` — `sourceIPs` (an `aws:SourceIp` CIDR allowlist) and + `secureTransportOnly` (require TLS). +- `spec.rawStatements` — an escape hatch of raw S3 policy statements + (`effect`/`actions`/`resources`/`conditions`) merged for this grant's + principal. When set, `level`, `actions`, `paths` and `conditions` are ignored; + resources without an `arn:` prefix are treated as bucket-relative key prefixes. + +RGW honours S3 bucket policy on **Reef 18.2+ / Squid**; condition-key support is +a subset of AWS, so validate exotic conditions against your cluster. + The `Bucket` controller renders the policy as the **union of every ready `BucketAccess`** that targets it, so the result is convergent regardless of the order objects are created or deleted. It watches `BucketAccess` and diff --git a/api/v1alpha1/bucketaccess_types.go b/api/v1alpha1/bucketaccess_types.go index 399a4c8..f13abb9 100644 --- a/api/v1alpha1/bucketaccess_types.go +++ b/api/v1alpha1/bucketaccess_types.go @@ -45,6 +45,72 @@ type BucketAccessSpec struct { // dedicated user it creates (UserRef empty). Defaults to "-rgw". // +optional SecretName string `json:"secretName,omitempty"` + + // Paths optionally scopes object-level access to these key prefixes within + // the bucket; each becomes the resource "/*". Empty grants + // the whole bucket. The bucket-level ListBucket action always applies to the + // whole bucket. Ignored when RawStatements is set. + // +optional + Paths []string `json:"paths,omitempty"` + + // Actions optionally overrides the S3 actions granted by Level. When set, + // exactly these actions are granted, on the bucket and its (optionally + // prefixed) objects. Ignored when RawStatements is set. + // +optional + Actions []string `json:"actions,omitempty"` + + // Conditions optionally restricts when the grant applies (e.g. source IPs, + // TLS required). Ignored when RawStatements is set. + // +optional + Conditions *AccessConditions `json:"conditions,omitempty"` + + // RawStatements is an escape hatch for arbitrary S3 policy statements, merged + // into the bucket policy for this grant's principal. When set, Level, + // Actions, Paths and Conditions on this object are ignored; the operator only + // fills in the Principal (this grant's user) when a statement omits one. + // +optional + RawStatements []PolicyStatement `json:"rawStatements,omitempty"` +} + +// AccessConditions restricts when a grant applies. Each field maps to an S3 +// policy condition and, when several are set, all must hold (they are AND'd). +type AccessConditions struct { + // SourceIPs restricts the grant to requests from these CIDRs (or single + // addresses), via the S3 aws:SourceIp condition. + // +optional + SourceIPs []string `json:"sourceIPs,omitempty"` + + // SecureTransportOnly requires the request to use TLS, via the S3 + // aws:SecureTransport condition. + // +optional + SecureTransportOnly bool `json:"secureTransportOnly,omitempty"` +} + +// PolicyStatement is a raw S3 bucket-policy statement, exposed for grants that +// need control beyond Level/Actions/Paths/Conditions. +type PolicyStatement struct { + // Sid is an optional statement id. The operator derives one when empty. + // +optional + Sid string `json:"sid,omitempty"` + + // Effect is Allow or Deny. Defaults to Allow. + // +kubebuilder:validation:Enum=Allow;Deny + // +kubebuilder:default=Allow + // +optional + Effect string `json:"effect,omitempty"` + + // Actions are the S3 actions the statement covers (e.g. s3:GetObject). + Actions []string `json:"actions"` + + // Resources are S3 resource ARNs, or bucket-relative key prefixes when they + // do not start with "arn:". Empty means the whole bucket and its objects. + // +optional + Resources []string `json:"resources,omitempty"` + + // Conditions is the raw S3 condition block: operator -> condition key -> + // values, e.g. {"IpAddress": {"aws:SourceIp": ["10.0.0.0/8"]}}. + // +optional + Conditions map[string]map[string][]string `json:"conditions,omitempty"` } // BucketAccessStatus reports observed grant state. diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index ba1499f..11735d8 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -9,6 +9,26 @@ import ( runtime "k8s.io/apimachinery/pkg/runtime" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AccessConditions) DeepCopyInto(out *AccessConditions) { + *out = *in + if in.SourceIPs != nil { + in, out := &in.SourceIPs, &out.SourceIPs + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AccessConditions. +func (in *AccessConditions) DeepCopy() *AccessConditions { + if in == nil { + return nil + } + out := new(AccessConditions) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Bucket) DeepCopyInto(out *Bucket) { *out = *in @@ -41,7 +61,7 @@ func (in *BucketAccess) DeepCopyInto(out *BucketAccess) { *out = *in out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - out.Spec = in.Spec + in.Spec.DeepCopyInto(&out.Spec) in.Status.DeepCopyInto(&out.Status) } @@ -98,6 +118,28 @@ func (in *BucketAccessList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *BucketAccessSpec) DeepCopyInto(out *BucketAccessSpec) { *out = *in + if in.Paths != nil { + in, out := &in.Paths, &out.Paths + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.Actions != nil { + in, out := &in.Actions, &out.Actions + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = new(AccessConditions) + (*in).DeepCopyInto(*out) + } + if in.RawStatements != nil { + in, out := &in.RawStatements, &out.RawStatements + *out = make([]PolicyStatement, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BucketAccessSpec. @@ -349,6 +391,58 @@ func (in *ObjectStoreUserStatus) DeepCopy() *ObjectStoreUserStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PolicyStatement) DeepCopyInto(out *PolicyStatement) { + *out = *in + if in.Actions != nil { + in, out := &in.Actions, &out.Actions + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.Resources != nil { + in, out := &in.Resources, &out.Resources + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make(map[string]map[string][]string, len(*in)) + for key, val := range *in { + var outVal map[string][]string + if val == nil { + (*out)[key] = nil + } else { + inVal := (*in)[key] + in, out := &inVal, &outVal + *out = make(map[string][]string, len(*in)) + for key, val := range *in { + var outVal []string + if val == nil { + (*out)[key] = nil + } else { + inVal := (*in)[key] + in, out := &inVal, &outVal + *out = make([]string, len(*in)) + copy(*out, *in) + } + (*out)[key] = outVal + } + } + (*out)[key] = outVal + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PolicyStatement. +func (in *PolicyStatement) DeepCopy() *PolicyStatement { + if in == nil { + return nil + } + out := new(PolicyStatement) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Quota) DeepCopyInto(out *Quota) { *out = *in diff --git a/config/crd/bases/ceph.unkin.net_bucketaccesses.yaml b/config/crd/bases/ceph.unkin.net_bucketaccesses.yaml index e283c8e..8b57c0f 100644 --- a/config/crd/bases/ceph.unkin.net_bucketaccesses.yaml +++ b/config/crd/bases/ceph.unkin.net_bucketaccesses.yaml @@ -60,10 +60,36 @@ spec: operator provisions a dedicated user for this grant and writes its keys into a Secret; otherwise it grants an existing ObjectStoreUser. properties: + actions: + description: |- + Actions optionally overrides the S3 actions granted by Level. When set, + exactly these actions are granted, on the bucket and its (optionally + prefixed) objects. Ignored when RawStatements is set. + items: + type: string + type: array bucketRef: description: BucketRef names the Bucket (in this namespace) to grant access to. type: string + conditions: + description: |- + Conditions optionally restricts when the grant applies (e.g. source IPs, + TLS required). Ignored when RawStatements is set. + properties: + secureTransportOnly: + description: |- + SecureTransportOnly requires the request to use TLS, via the S3 + aws:SecureTransport condition. + type: boolean + sourceIPs: + description: |- + SourceIPs restricts the grant to requests from these CIDRs (or single + addresses), via the S3 aws:SourceIp condition. + items: + type: string + type: array + type: object level: description: Level is the access level to grant. enum: @@ -71,6 +97,65 @@ spec: - read-write - full type: string + paths: + description: |- + Paths optionally scopes object-level access to these key prefixes within + the bucket; each becomes the resource "/*". Empty grants + the whole bucket. The bucket-level ListBucket action always applies to the + whole bucket. Ignored when RawStatements is set. + items: + type: string + type: array + rawStatements: + description: |- + RawStatements is an escape hatch for arbitrary S3 policy statements, merged + into the bucket policy for this grant's principal. When set, Level, + Actions, Paths and Conditions on this object are ignored; the operator only + fills in the Principal (this grant's user) when a statement omits one. + items: + description: |- + PolicyStatement is a raw S3 bucket-policy statement, exposed for grants that + need control beyond Level/Actions/Paths/Conditions. + properties: + actions: + description: Actions are the S3 actions the statement covers + (e.g. s3:GetObject). + items: + type: string + type: array + conditions: + additionalProperties: + additionalProperties: + items: + type: string + type: array + type: object + description: |- + Conditions is the raw S3 condition block: operator -> condition key -> + values, e.g. {"IpAddress": {"aws:SourceIp": ["10.0.0.0/8"]}}. + type: object + effect: + default: Allow + description: Effect is Allow or Deny. Defaults to Allow. + enum: + - Allow + - Deny + type: string + resources: + description: |- + Resources are S3 resource ARNs, or bucket-relative key prefixes when they + do not start with "arn:". Empty means the whole bucket and its objects. + items: + type: string + type: array + sid: + description: Sid is an optional statement id. The operator derives + one when empty. + type: string + required: + - actions + type: object + type: array secretName: description: |- SecretName is the Secret the operator writes credentials into for the diff --git a/config/crd/bases/ceph.unkin.net_objectstoreusers.yaml b/config/crd/bases/ceph.unkin.net_objectstoreusers.yaml index a9ec44e..bef7c4f 100644 --- a/config/crd/bases/ceph.unkin.net_objectstoreusers.yaml +++ b/config/crd/bases/ceph.unkin.net_objectstoreusers.yaml @@ -52,7 +52,7 @@ spec: spec: description: |- ObjectStoreUserSpec defines a Ceph RGW (S3) user. The operator creates the - user through the Ceph dashboard API and writes its generated access/secret + user through the radosgw Admin Ops API and writes its generated access/secret key pair into a Kubernetes Secret. The key material is never stored on the resource itself. properties: diff --git a/config/crd/install.yaml b/config/crd/install.yaml index 64614b0..20f13fa 100644 --- a/config/crd/install.yaml +++ b/config/crd/install.yaml @@ -61,10 +61,36 @@ spec: operator provisions a dedicated user for this grant and writes its keys into a Secret; otherwise it grants an existing ObjectStoreUser. properties: + actions: + description: |- + Actions optionally overrides the S3 actions granted by Level. When set, + exactly these actions are granted, on the bucket and its (optionally + prefixed) objects. Ignored when RawStatements is set. + items: + type: string + type: array bucketRef: description: BucketRef names the Bucket (in this namespace) to grant access to. type: string + conditions: + description: |- + Conditions optionally restricts when the grant applies (e.g. source IPs, + TLS required). Ignored when RawStatements is set. + properties: + secureTransportOnly: + description: |- + SecureTransportOnly requires the request to use TLS, via the S3 + aws:SecureTransport condition. + type: boolean + sourceIPs: + description: |- + SourceIPs restricts the grant to requests from these CIDRs (or single + addresses), via the S3 aws:SourceIp condition. + items: + type: string + type: array + type: object level: description: Level is the access level to grant. enum: @@ -72,6 +98,65 @@ spec: - read-write - full type: string + paths: + description: |- + Paths optionally scopes object-level access to these key prefixes within + the bucket; each becomes the resource "/*". Empty grants + the whole bucket. The bucket-level ListBucket action always applies to the + whole bucket. Ignored when RawStatements is set. + items: + type: string + type: array + rawStatements: + description: |- + RawStatements is an escape hatch for arbitrary S3 policy statements, merged + into the bucket policy for this grant's principal. When set, Level, + Actions, Paths and Conditions on this object are ignored; the operator only + fills in the Principal (this grant's user) when a statement omits one. + items: + description: |- + PolicyStatement is a raw S3 bucket-policy statement, exposed for grants that + need control beyond Level/Actions/Paths/Conditions. + properties: + actions: + description: Actions are the S3 actions the statement covers + (e.g. s3:GetObject). + items: + type: string + type: array + conditions: + additionalProperties: + additionalProperties: + items: + type: string + type: array + type: object + description: |- + Conditions is the raw S3 condition block: operator -> condition key -> + values, e.g. {"IpAddress": {"aws:SourceIp": ["10.0.0.0/8"]}}. + type: object + effect: + default: Allow + description: Effect is Allow or Deny. Defaults to Allow. + enum: + - Allow + - Deny + type: string + resources: + description: |- + Resources are S3 resource ARNs, or bucket-relative key prefixes when they + do not start with "arn:". Empty means the whole bucket and its objects. + items: + type: string + type: array + sid: + description: Sid is an optional statement id. The operator derives + one when empty. + type: string + required: + - actions + type: object + type: array secretName: description: |- SecretName is the Secret the operator writes credentials into for the @@ -463,7 +548,7 @@ spec: spec: description: |- ObjectStoreUserSpec defines a Ceph RGW (S3) user. The operator creates the - user through the Ceph dashboard API and writes its generated access/secret + user through the radosgw Admin Ops API and writes its generated access/secret key pair into a Kubernetes Secret. The key material is never stored on the resource itself. properties: diff --git a/config/samples/04-access-fine-grained.yaml b/config/samples/04-access-fine-grained.yaml new file mode 100644 index 0000000..3e49d5a --- /dev/null +++ b/config/samples/04-access-fine-grained.yaml @@ -0,0 +1,78 @@ +# Fine-grained grants. Each of these refines the coarse read-only/read-write/full +# levels with prefix scoping, action overrides, conditions, or raw statements. + +# 1. Prefix-scoped read-write: this workload may read/write objects only under +# the "uploads/" and "tmp/" key prefixes (bucket-level ListBucket still spans +# the whole bucket). +apiVersion: ceph.unkin.net/v1alpha1 +kind: BucketAccess +metadata: + name: app-data-uploader + namespace: default +spec: + bucketRef: app-data + level: read-write + secretName: app-data-uploader-rgw + paths: + - uploads/ + - tmp/ +--- +# 2. Read-only from inside the cluster only: restrict the grant to a source CIDR +# and require TLS. +apiVersion: ceph.unkin.net/v1alpha1 +kind: BucketAccess +metadata: + name: app-data-internal-ro + namespace: default +spec: + bucketRef: app-data + level: read-only + secretName: app-data-internal-ro-rgw + conditions: + sourceIPs: + - 10.0.0.0/8 + secureTransportOnly: true +--- +# 3. Explicit action set: grant exactly these actions instead of a level's +# canned set (level is still required but its actions are ignored). +apiVersion: ceph.unkin.net/v1alpha1 +kind: BucketAccess +metadata: + name: app-data-getput + namespace: default +spec: + bucketRef: app-data + level: read-only + secretName: app-data-getput-rgw + actions: + - s3:GetObject + - s3:PutObject +--- +# 4. Raw statements escape hatch: full control over the policy statement. Level, +# actions, paths and conditions are ignored; the operator only injects the +# Principal (this grant's user). Resources without an "arn:" prefix are +# treated as bucket-relative key prefixes. +apiVersion: ceph.unkin.net/v1alpha1 +kind: BucketAccess +metadata: + name: app-data-raw + namespace: default +spec: + bucketRef: app-data + level: read-only + secretName: app-data-raw-rgw + rawStatements: + - effect: Allow + actions: + - s3:GetObject + resources: + - public/ + - effect: Deny + actions: + - s3:DeleteObject + resources: + - locked/ + conditions: + Bool: + aws:SecureTransport: + - "false" diff --git a/internal/ceph/policy.go b/internal/ceph/policy.go index 0e14a9a..ecdb048 100644 --- a/internal/ceph/policy.go +++ b/internal/ceph/policy.go @@ -3,6 +3,7 @@ package ceph import ( "encoding/json" "sort" + "strconv" "strings" ) @@ -14,10 +15,40 @@ const ( LevelFull = "full" ) -// Grant couples an RGW user id with the access level to grant it on a bucket. +// GrantConditions restricts when a grant's statements apply. The zero value adds +// no conditions. +type GrantConditions struct { + // SourceIPs restricts the grant to these CIDRs (S3 aws:SourceIp). + SourceIPs []string + // SecureTransportOnly requires TLS (S3 aws:SecureTransport). + SecureTransportOnly bool +} + +// RawStatement is a caller-supplied S3 policy statement for a grant. +type RawStatement struct { + Sid string + Effect string + Actions []string + Resources []string + Condition map[string]map[string][]string +} + +// Grant couples an RGW user id with the access it should have on a bucket. The +// simple form is a Level; Paths, Actions and Conditions refine it, and Raw +// replaces it entirely with caller-supplied statements. type Grant struct { UID string Level string + // Paths scopes object-level access to these key prefixes; empty = whole + // bucket. + Paths []string + // Actions overrides the level's action set; empty = derive from Level. + Actions []string + // Conditions optionally restricts when the grant applies. + Conditions *GrantConditions + // Raw, when non-empty, replaces Level/Actions/Paths/Conditions with these + // statements (the operator still fills in a Principal when one is omitted). + Raw []RawStatement } type policyDocument struct { @@ -26,11 +57,12 @@ type policyDocument struct { } type policyStatement struct { - Sid string `json:"Sid"` - Effect string `json:"Effect"` - Principal map[string][]string `json:"Principal"` - Action []string `json:"Action"` - Resource []string `json:"Resource"` + Sid string `json:"Sid,omitempty"` + Effect string `json:"Effect"` + Principal map[string][]string `json:"Principal,omitempty"` + Action []string `json:"Action"` + Resource []string `json:"Resource"` + Condition map[string]map[string][]string `json:"Condition,omitempty"` } // bucket-level and object-level S3 actions per access level. @@ -68,7 +100,7 @@ var objectActions = map[string][]string{ } // BuildBucketPolicy renders a deterministic S3 bucket policy granting each -// principal its requested level. It returns "" when there are no grants so the +// principal its requested access. It returns "" when there are no grants so the // caller can clear the policy. func BuildBucketPolicy(bucket string, grants []Grant) (string, error) { if len(grants) == 0 { @@ -85,38 +117,10 @@ func BuildBucketPolicy(bucket string, grants []Grant) (string, error) { }) bucketARN := "arn:aws:s3:::" + bucket - objectARN := bucketARN + "/*" doc := policyDocument{Version: "2012-10-17"} for _, g := range sorted { - principal := map[string][]string{"AWS": {"arn:aws:iam:::user/" + g.UID}} - switch g.Level { - case LevelFull: - doc.Statement = append(doc.Statement, policyStatement{ - Sid: sid("full", g.UID), - Effect: "Allow", - Principal: principal, - Action: []string{"s3:*"}, - Resource: []string{bucketARN, objectARN}, - }) - default: - doc.Statement = append(doc.Statement, - policyStatement{ - Sid: sid(g.Level+"-bkt", g.UID), - Effect: "Allow", - Principal: principal, - Action: bucketActions[g.Level], - Resource: []string{bucketARN}, - }, - policyStatement{ - Sid: sid(g.Level+"-obj", g.UID), - Effect: "Allow", - Principal: principal, - Action: objectActions[g.Level], - Resource: []string{objectARN}, - }, - ) - } + doc.Statement = append(doc.Statement, statementsForGrant(bucketARN, g)...) } b, err := json.Marshal(doc) @@ -126,6 +130,133 @@ func BuildBucketPolicy(bucket string, grants []Grant) (string, error) { return string(b), nil } +// statementsForGrant renders the policy statements for a single grant. +func statementsForGrant(bucketARN string, g Grant) []policyStatement { + principal := map[string][]string{"AWS": {"arn:aws:iam:::user/" + g.UID}} + + if len(g.Raw) > 0 { + out := make([]policyStatement, 0, len(g.Raw)) + for i, rs := range g.Raw { + st := policyStatement{ + Sid: firstNonEmpty(rs.Sid, sid("raw", g.UID)+strconv.Itoa(i)), + Effect: firstNonEmpty(rs.Effect, "Allow"), + Principal: principal, + Action: rs.Actions, + Resource: resolveResources(bucketARN, rs.Resources), + Condition: rs.Condition, + } + out = append(out, st) + } + return out + } + + cond := buildCondition(g.Conditions) + objectARNs := objectResources(bucketARN, g.Paths) + + if len(g.Actions) > 0 { + return []policyStatement{{ + Sid: sid("custom", g.UID), + Effect: "Allow", + Principal: principal, + Action: g.Actions, + Resource: append([]string{bucketARN}, objectARNs...), + Condition: cond, + }} + } + + if g.Level == LevelFull { + return []policyStatement{{ + Sid: sid("full", g.UID), + Effect: "Allow", + Principal: principal, + Action: []string{"s3:*"}, + Resource: append([]string{bucketARN}, objectARNs...), + Condition: cond, + }} + } + + return []policyStatement{ + { + Sid: sid(g.Level+"-bkt", g.UID), + Effect: "Allow", + Principal: principal, + Action: bucketActions[g.Level], + Resource: []string{bucketARN}, + Condition: cond, + }, + { + Sid: sid(g.Level+"-obj", g.UID), + Effect: "Allow", + Principal: principal, + Action: objectActions[g.Level], + Resource: objectARNs, + Condition: cond, + }, + } +} + +// objectResources renders the object-level resource ARNs for a grant: the whole +// bucket ("/*") when no paths are given, or one "/*" per +// prefix (deduplicated and sorted for determinism). +func objectResources(bucketARN string, paths []string) []string { + if len(paths) == 0 { + return []string{bucketARN + "/*"} + } + seen := map[string]struct{}{} + out := make([]string, 0, len(paths)) + for _, p := range paths { + p = strings.TrimPrefix(p, "/") + arn := bucketARN + "/" + p + "*" + if _, dup := seen[arn]; dup { + continue + } + seen[arn] = struct{}{} + out = append(out, arn) + } + sort.Strings(out) + return out +} + +// resolveResources renders raw-statement resources: entries that already look +// like ARNs pass through verbatim; bucket-relative prefixes become +// "/*". An empty list defaults to the whole bucket and objects. +func resolveResources(bucketARN string, resources []string) []string { + if len(resources) == 0 { + return []string{bucketARN, bucketARN + "/*"} + } + out := make([]string, 0, len(resources)) + for _, r := range resources { + switch { + case strings.HasPrefix(r, "arn:"): + out = append(out, r) + case r == "" || r == "/": + out = append(out, bucketARN+"/*") + default: + out = append(out, bucketARN+"/"+strings.TrimPrefix(r, "/")+"*") + } + } + return out +} + +// buildCondition renders the S3 condition block for a grant, or nil when there +// is nothing to add. +func buildCondition(c *GrantConditions) map[string]map[string][]string { + if c == nil { + return nil + } + cond := map[string]map[string][]string{} + if len(c.SourceIPs) > 0 { + cond["IpAddress"] = map[string][]string{"aws:SourceIp": c.SourceIPs} + } + if c.SecureTransportOnly { + cond["Bool"] = map[string][]string{"aws:SecureTransport": {"true"}} + } + if len(cond) == 0 { + return nil + } + return cond +} + // sid builds a policy statement id that only contains characters S3 accepts. func sid(prefix, uid string) string { var b strings.Builder diff --git a/internal/ceph/policy_test.go b/internal/ceph/policy_test.go index 0ea18c7..fb1bff7 100644 --- a/internal/ceph/policy_test.go +++ b/internal/ceph/policy_test.go @@ -88,3 +88,155 @@ func TestBuildBucketPolicyStructure(t *testing.T) { t.Fatal("reader principal ARN missing") } } + +// parsedPolicy is a fuller parse of a rendered policy for the fine-grained tests. +type parsedPolicy struct { + Statement []struct { + Sid string `json:"Sid"` + Effect string `json:"Effect"` + Principal map[string][]string `json:"Principal"` + Action []string `json:"Action"` + Resource []string `json:"Resource"` + Condition map[string]map[string][]string `json:"Condition"` + } `json:"Statement"` +} + +func parsePolicy(t *testing.T, raw string) parsedPolicy { + t.Helper() + var doc parsedPolicy + if err := json.Unmarshal([]byte(raw), &doc); err != nil { + t.Fatalf("policy is not valid JSON: %v\n%s", err, raw) + } + return doc +} + +func TestBuildBucketPolicyPaths(t *testing.T) { + raw, err := BuildBucketPolicy("data", []Grant{ + {UID: "reader", Level: LevelReadOnly, Paths: []string{"team-a/", "/shared/inbox/"}}, + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + doc := parsePolicy(t, raw) + + var objResources []string + for _, s := range doc.Statement { + for _, a := range s.Action { + if a == "s3:GetObject" { + objResources = s.Resource + } + } + } + want := map[string]bool{ + "arn:aws:s3:::data/shared/inbox/*": false, + "arn:aws:s3:::data/team-a/*": false, + } + if len(objResources) != len(want) { + t.Fatalf("expected %d object resources, got %v", len(want), objResources) + } + for _, r := range objResources { + if _, ok := want[r]; !ok { + t.Fatalf("unexpected object resource %q (leading slash not trimmed?)", r) + } + want[r] = true + } + for r, seen := range want { + if !seen { + t.Fatalf("missing object resource %q", r) + } + } +} + +func TestBuildBucketPolicyActionsOverride(t *testing.T) { + raw, err := BuildBucketPolicy("data", []Grant{ + {UID: "svc", Level: LevelReadOnly, Actions: []string{"s3:GetObject", "s3:PutObject"}}, + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + doc := parsePolicy(t, raw) + if len(doc.Statement) != 1 { + t.Fatalf("expected a single custom-action statement, got %d", len(doc.Statement)) + } + s := doc.Statement[0] + if len(s.Action) != 2 || s.Action[0] != "s3:GetObject" || s.Action[1] != "s3:PutObject" { + t.Fatalf("actions not taken verbatim: %v", s.Action) + } + // Custom-action statement lists both the bucket and object resources. + if len(s.Resource) != 2 || s.Resource[0] != "arn:aws:s3:::data" || s.Resource[1] != "arn:aws:s3:::data/*" { + t.Fatalf("unexpected resources: %v", s.Resource) + } +} + +func TestBuildBucketPolicyConditions(t *testing.T) { + raw, err := BuildBucketPolicy("data", []Grant{ + {UID: "reader", Level: LevelReadOnly, Conditions: &GrantConditions{ + SourceIPs: []string{"10.0.0.0/8"}, + SecureTransportOnly: true, + }}, + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + doc := parsePolicy(t, raw) + for _, s := range doc.Statement { + if s.Condition == nil { + t.Fatalf("statement %q missing condition block", s.Sid) + } + if ip := s.Condition["IpAddress"]["aws:SourceIp"]; len(ip) != 1 || ip[0] != "10.0.0.0/8" { + t.Fatalf("unexpected SourceIp condition: %v", s.Condition["IpAddress"]) + } + if tls := s.Condition["Bool"]["aws:SecureTransport"]; len(tls) != 1 || tls[0] != "true" { + t.Fatalf("unexpected SecureTransport condition: %v", s.Condition["Bool"]) + } + } +} + +func TestBuildBucketPolicyRawStatements(t *testing.T) { + raw, err := BuildBucketPolicy("data", []Grant{ + {UID: "svc", Level: LevelReadOnly, Raw: []RawStatement{ + { + Effect: "Deny", + Actions: []string{"s3:DeleteObject"}, + Resources: []string{"locked/", "arn:aws:s3:::other/*"}, + }, + }}, + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + doc := parsePolicy(t, raw) + if len(doc.Statement) != 1 { + t.Fatalf("expected 1 raw statement, got %d", len(doc.Statement)) + } + s := doc.Statement[0] + if s.Effect != "Deny" { + t.Fatalf("raw effect not honoured: %q", s.Effect) + } + // The operator fills in the principal; bucket-relative prefixes are expanded + // while explicit ARNs pass through. + if len(s.Principal["AWS"]) != 1 || !strings.HasSuffix(s.Principal["AWS"][0], "user/svc") { + t.Fatalf("raw statement principal not injected: %v", s.Principal) + } + if len(s.Resource) != 2 || s.Resource[0] != "arn:aws:s3:::data/locked/*" || s.Resource[1] != "arn:aws:s3:::other/*" { + t.Fatalf("unexpected raw resources: %v", s.Resource) + } +} + +func TestBuildBucketPolicyFineGrainedDeterministic(t *testing.T) { + grants := []Grant{ + {UID: "reader", Level: LevelReadOnly, Paths: []string{"a/", "b/"}}, + {UID: "svc", Level: LevelReadWrite, Conditions: &GrantConditions{SecureTransportOnly: true}}, + } + a, err := BuildBucketPolicy("data", grants) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + b, err := BuildBucketPolicy("data", []Grant{grants[1], grants[0]}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if a != b { + t.Fatalf("fine-grained policy is order-dependent:\n a=%s\n b=%s", a, b) + } +} diff --git a/internal/controller/bucket_controller.go b/internal/controller/bucket_controller.go index f7d1d43..4542bd1 100644 --- a/internal/controller/bucket_controller.go +++ b/internal/controller/bucket_controller.go @@ -2,6 +2,7 @@ package controller import ( "context" + "encoding/json" "fmt" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -176,17 +177,52 @@ func (r *BucketReconciler) collectGrants(ctx context.Context, namespace, bucketR if ba.Status.UID == "" { continue } - key := ba.Status.UID + "|" + string(ba.Spec.Level) + g := grantFromAccess(ba.Status.UID, ba) + key := grantKey(g) if _, dup := seen[key]; dup { continue } seen[key] = struct{}{} principals[ba.Status.UID] = struct{}{} - grants = append(grants, ceph.Grant{UID: ba.Status.UID, Level: string(ba.Spec.Level)}) + grants = append(grants, g) } return grants, len(principals), nil } +// grantFromAccess translates a BucketAccess spec into the ceph grant model, +// carrying the fine-grained scoping (paths, actions, conditions, raw statements). +func grantFromAccess(uid string, ba *v1alpha1.BucketAccess) ceph.Grant { + g := ceph.Grant{ + UID: uid, + Level: string(ba.Spec.Level), + Paths: ba.Spec.Paths, + Actions: ba.Spec.Actions, + } + if c := ba.Spec.Conditions; c != nil { + g.Conditions = &ceph.GrantConditions{ + SourceIPs: c.SourceIPs, + SecureTransportOnly: c.SecureTransportOnly, + } + } + for _, s := range ba.Spec.RawStatements { + g.Raw = append(g.Raw, ceph.RawStatement{ + Sid: s.Sid, + Effect: s.Effect, + Actions: s.Actions, + Resources: s.Resources, + Condition: s.Conditions, + }) + } + return g +} + +// grantKey is a stable fingerprint of a grant used to collapse duplicate +// BucketAccess objects that would render identical policy statements. +func grantKey(g ceph.Grant) string { + b, _ := json.Marshal(g) + return string(b) +} + func (r *BucketReconciler) pending(ctx context.Context, b *v1alpha1.Bucket, reason, msg string) (ctrl.Result, error) { b.Status.Phase = "Pending" b.Status.ObservedGeneration = b.Generation