From 34ab1b5483a99c449960a000bc9d1db744ef9f6f Mon Sep 17 00:00:00 2001 From: Jose Szychowski Date: Wed, 25 Mar 2026 10:03:58 -0300 Subject: [PATCH 01/20] feat: migraet machineAccountKey from IAM APIGroup to Identity APIGroup This change is made to take advantage of the already custom storage layer of the identity APIGroup --- .../iam.miloapis.com_machineaccountkeys.yaml | 153 -------------- config/crd/bases/iam/kustomization.yaml | 1 - .../resources-metrics/iam/kustomization.yaml | 1 - .../machine_account_keys.yaml | 4 +- config/resources-metrics/kustomization.yaml | 1 + pkg/apis/iam/v1alpha1/register.go | 2 - .../iam/v1alpha1/zz_generated.deepcopy.go | 100 --------- .../v1alpha1/machineaccountkey_types.go | 16 +- pkg/apis/identity/v1alpha1/register.go | 2 + .../v1alpha1/zz_generated.deepcopy.go | 101 +++++++++ .../identity/v1alpha1/zz_generated.openapi.go | 200 +++++++++++++++++- 11 files changed, 315 insertions(+), 266 deletions(-) delete mode 100644 config/crd/bases/iam/iam.miloapis.com_machineaccountkeys.yaml rename config/resources-metrics/{iam => identity}/machine_account_keys.yaml (85%) rename pkg/apis/{iam => identity}/v1alpha1/machineaccountkey_types.go (80%) diff --git a/config/crd/bases/iam/iam.miloapis.com_machineaccountkeys.yaml b/config/crd/bases/iam/iam.miloapis.com_machineaccountkeys.yaml deleted file mode 100644 index e8cb7935..00000000 --- a/config/crd/bases/iam/iam.miloapis.com_machineaccountkeys.yaml +++ /dev/null @@ -1,153 +0,0 @@ ---- -apiVersion: apiextensions.k8s.io/v1 -kind: CustomResourceDefinition -metadata: - annotations: - controller-gen.kubebuilder.io/version: v0.18.0 - name: machineaccountkeys.iam.miloapis.com -spec: - group: iam.miloapis.com - names: - kind: MachineAccountKey - listKind: MachineAccountKeyList - plural: machineaccountkeys - singular: machineaccountkey - scope: Namespaced - versions: - - additionalPrinterColumns: - - jsonPath: .spec.machineAccountName - name: Machine Account - type: string - - jsonPath: .spec.expirationDate - name: Expiration Date - type: string - - jsonPath: .status.conditions[?(@.type=='Ready')].status - name: Ready - type: string - - jsonPath: .metadata.creationTimestamp - name: Age - type: date - name: v1alpha1 - schema: - openAPIV3Schema: - description: MachineAccountKey is the Schema for the machineaccountkeys API - properties: - apiVersion: - description: |- - APIVersion defines the versioned schema of this representation of an object. - Servers should convert recognized schemas to the latest internal value, and - may reject unrecognized values. - More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources - type: string - kind: - description: |- - Kind is a string value representing the REST resource this object represents. - Servers may infer this from the endpoint the client submits requests to. - Cannot be updated. - In CamelCase. - More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds - type: string - metadata: - type: object - spec: - description: MachineAccountKeySpec defines the desired state of MachineAccountKey - properties: - expirationDate: - description: |- - ExpirationDate is the date and time when the MachineAccountKey will expire. - If not specified, the MachineAccountKey will never expire. - format: date-time - type: string - machineAccountName: - description: MachineAccountName is the name of the MachineAccount - that owns this key. - type: string - publicKey: - description: |- - PublicKey is the public key of the MachineAccountKey. - If not specified, the MachineAccountKey will be created with an auto-generated public key. - type: string - required: - - machineAccountName - type: object - status: - description: MachineAccountKeyStatus defines the observed state of MachineAccountKey - properties: - authProviderKeyId: - description: |- - AuthProviderKeyID is the unique identifier for the key in the auth provider. - This field is populated by the controller after the key is created in the auth provider. - For example, when using Zitadel, a typical value might be: "326102453042806786" - type: string - conditions: - default: - - lastTransitionTime: "1970-01-01T00:00:00Z" - message: Waiting for control plane to reconcile - reason: Unknown - status: Unknown - type: Ready - description: Conditions provide conditions that represent the current - status of the MachineAccountKey. - items: - description: Condition contains details for one aspect of the current - state of this API Resource. - properties: - lastTransitionTime: - description: |- - lastTransitionTime is the last time the condition transitioned from one status to another. - This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. - format: date-time - type: string - message: - description: |- - message is a human readable message indicating details about the transition. - This may be an empty string. - maxLength: 32768 - type: string - observedGeneration: - description: |- - observedGeneration represents the .metadata.generation that the condition was set based upon. - For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date - with respect to the current state of the instance. - format: int64 - minimum: 0 - type: integer - reason: - description: |- - reason contains a programmatic identifier indicating the reason for the condition's last transition. - Producers of specific condition types may define expected values and meanings for this field, - and whether the values are considered a guaranteed API. - The value should be a CamelCase string. - This field may not be empty. - maxLength: 1024 - minLength: 1 - pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ - type: string - status: - description: status of the condition, one of True, False, Unknown. - enum: - - "True" - - "False" - - Unknown - type: string - type: - description: type of condition in CamelCase or in foo.example.com/CamelCase. - maxLength: 316 - pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ - type: string - required: - - lastTransitionTime - - message - - reason - - status - - type - type: object - type: array - type: object - type: object - selectableFields: - - jsonPath: .spec.machineAccountName - served: true - storage: true - subresources: - status: {} diff --git a/config/crd/bases/iam/kustomization.yaml b/config/crd/bases/iam/kustomization.yaml index 797c8a51..366abfce 100644 --- a/config/crd/bases/iam/kustomization.yaml +++ b/config/crd/bases/iam/kustomization.yaml @@ -7,7 +7,6 @@ resources: - iam.miloapis.com_protectedresources.yaml - iam.miloapis.com_users.yaml - iam.miloapis.com_userinvitations.yaml -- iam.miloapis.com_machineaccountkeys.yaml - iam.miloapis.com_userpreferences.yaml - iam.miloapis.com_userdeactivations.yaml - iam.miloapis.com_platforminvitations.yaml diff --git a/config/resources-metrics/iam/kustomization.yaml b/config/resources-metrics/iam/kustomization.yaml index 26e37a8a..06e77315 100644 --- a/config/resources-metrics/iam/kustomization.yaml +++ b/config/resources-metrics/iam/kustomization.yaml @@ -8,7 +8,6 @@ configMapGenerator: - groups.yaml - group_memberships.yaml - machine_accounts.yaml - - machine_account_keys.yaml - policy_bindings.yaml - roles.yaml - user_invitations.yaml diff --git a/config/resources-metrics/iam/machine_account_keys.yaml b/config/resources-metrics/identity/machine_account_keys.yaml similarity index 85% rename from config/resources-metrics/iam/machine_account_keys.yaml rename to config/resources-metrics/identity/machine_account_keys.yaml index 9bd7e3f0..9ebbc86f 100644 --- a/config/resources-metrics/iam/machine_account_keys.yaml +++ b/config/resources-metrics/identity/machine_account_keys.yaml @@ -2,7 +2,7 @@ kind: CustomResourceStateMetrics spec: resources: - groupVersionKind: - group: "iam.miloapis.com" + group: "identity.miloapis.com" kind: "MachineAccountKey" version: "v1alpha1" labelsFromPath: @@ -20,4 +20,4 @@ spec: each: type: Gauge gauge: - path: [metadata, creationTimestamp] \ No newline at end of file + path: [metadata, creationTimestamp] diff --git a/config/resources-metrics/kustomization.yaml b/config/resources-metrics/kustomization.yaml index 3284a57e..ebc2a4ae 100644 --- a/config/resources-metrics/kustomization.yaml +++ b/config/resources-metrics/kustomization.yaml @@ -3,5 +3,6 @@ kind: Component components: - iam/ + - identity/ - resources-manager/ - infrastructure/ diff --git a/pkg/apis/iam/v1alpha1/register.go b/pkg/apis/iam/v1alpha1/register.go index 828278ce..5a00f729 100644 --- a/pkg/apis/iam/v1alpha1/register.go +++ b/pkg/apis/iam/v1alpha1/register.go @@ -35,8 +35,6 @@ func addKnownTypes(scheme *runtime.Scheme) error { &ProtectedResourceList{}, &MachineAccount{}, &MachineAccountList{}, - &MachineAccountKey{}, - &MachineAccountKeyList{}, &UserPreference{}, &UserPreferenceList{}, &UserDeactivation{}, diff --git a/pkg/apis/iam/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/iam/v1alpha1/zz_generated.deepcopy.go index 0218d4c2..fc6e024d 100644 --- a/pkg/apis/iam/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/iam/v1alpha1/zz_generated.deepcopy.go @@ -229,106 +229,6 @@ func (in *MachineAccount) DeepCopyObject() runtime.Object { return nil } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *MachineAccountKey) DeepCopyInto(out *MachineAccountKey) { - *out = *in - out.TypeMeta = in.TypeMeta - in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - in.Spec.DeepCopyInto(&out.Spec) - in.Status.DeepCopyInto(&out.Status) -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineAccountKey. -func (in *MachineAccountKey) DeepCopy() *MachineAccountKey { - if in == nil { - return nil - } - out := new(MachineAccountKey) - in.DeepCopyInto(out) - return out -} - -// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. -func (in *MachineAccountKey) DeepCopyObject() runtime.Object { - if c := in.DeepCopy(); c != nil { - return c - } - return nil -} - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *MachineAccountKeyList) DeepCopyInto(out *MachineAccountKeyList) { - *out = *in - out.TypeMeta = in.TypeMeta - in.ListMeta.DeepCopyInto(&out.ListMeta) - if in.Items != nil { - in, out := &in.Items, &out.Items - *out = make([]MachineAccountKey, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineAccountKeyList. -func (in *MachineAccountKeyList) DeepCopy() *MachineAccountKeyList { - if in == nil { - return nil - } - out := new(MachineAccountKeyList) - in.DeepCopyInto(out) - return out -} - -// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. -func (in *MachineAccountKeyList) DeepCopyObject() runtime.Object { - if c := in.DeepCopy(); c != nil { - return c - } - return nil -} - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *MachineAccountKeySpec) DeepCopyInto(out *MachineAccountKeySpec) { - *out = *in - if in.ExpirationDate != nil { - in, out := &in.ExpirationDate, &out.ExpirationDate - *out = (*in).DeepCopy() - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineAccountKeySpec. -func (in *MachineAccountKeySpec) DeepCopy() *MachineAccountKeySpec { - if in == nil { - return nil - } - out := new(MachineAccountKeySpec) - in.DeepCopyInto(out) - return out -} - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *MachineAccountKeyStatus) DeepCopyInto(out *MachineAccountKeyStatus) { - *out = *in - if in.Conditions != nil { - in, out := &in.Conditions, &out.Conditions - *out = make([]v1.Condition, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineAccountKeyStatus. -func (in *MachineAccountKeyStatus) DeepCopy() *MachineAccountKeyStatus { - if in == nil { - return nil - } - out := new(MachineAccountKeyStatus) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MachineAccountList) DeepCopyInto(out *MachineAccountList) { *out = *in diff --git a/pkg/apis/iam/v1alpha1/machineaccountkey_types.go b/pkg/apis/identity/v1alpha1/machineaccountkey_types.go similarity index 80% rename from pkg/apis/iam/v1alpha1/machineaccountkey_types.go rename to pkg/apis/identity/v1alpha1/machineaccountkey_types.go index 4c8e0b33..bce63926 100644 --- a/pkg/apis/iam/v1alpha1/machineaccountkey_types.go +++ b/pkg/apis/identity/v1alpha1/machineaccountkey_types.go @@ -7,6 +7,7 @@ import ( // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object // +kubebuilder:subresource:status // +kubebuilder:object:root=true +// +kubebuilder:storageversion // MachineAccountKey is the Schema for the machineaccountkeys API // +kubebuilder:printcolumn:name="Machine Account",type="string",JSONPath=".spec.machineAccountName" @@ -45,11 +46,24 @@ type MachineAccountKeyStatus struct { // AuthProviderKeyID is the unique identifier for the key in the auth provider. // This field is populated by the controller after the key is created in the auth provider. // For example, when using Zitadel, a typical value might be: "326102453042806786" - AuthProviderKeyID string `json:"authProviderKeyId,omitempty"` + AuthProviderKeyID string `json:"authProviderKeyID,omitempty"` + + // PrivateKey contains the PEM-encoded RSA private key generated during resource + // creation. This field is populated only in the creation response and is never + // persisted to etcd. Any value present on a GET or LIST response indicates a + // bug in the server implementation. + // + // Note: private key material will appear in API server audit logs for creation + // events. This matches the behavior of similar systems (GCP service account keys). + // + // +kubebuilder:validation:Optional + PrivateKey string `json:"privateKey,omitempty"` // Conditions provide conditions that represent the current status of the MachineAccountKey. // +kubebuilder:default={{type: "Ready", status: "Unknown", reason: "Unknown", message: "Waiting for control plane to reconcile", lastTransitionTime: "1970-01-01T00:00:00Z"}} // +kubebuilder:validation:Optional + // +listType=map + // +listMapKey=type Conditions []metav1.Condition `json:"conditions,omitempty"` } diff --git a/pkg/apis/identity/v1alpha1/register.go b/pkg/apis/identity/v1alpha1/register.go index 66560647..3c97347a 100644 --- a/pkg/apis/identity/v1alpha1/register.go +++ b/pkg/apis/identity/v1alpha1/register.go @@ -33,6 +33,8 @@ func addKnownTypes(scheme *runtime.Scheme) error { &SessionList{}, &UserIdentity{}, &UserIdentityList{}, + &MachineAccountKey{}, + &MachineAccountKeyList{}, ) metav1.AddToGroupVersion(scheme, SchemeGroupVersion) return nil diff --git a/pkg/apis/identity/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/identity/v1alpha1/zz_generated.deepcopy.go index 73588aed..d8b27588 100644 --- a/pkg/apis/identity/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/identity/v1alpha1/zz_generated.deepcopy.go @@ -5,9 +5,110 @@ package v1alpha1 import ( + "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *MachineAccountKey) DeepCopyInto(out *MachineAccountKey) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + in.Spec.DeepCopyInto(&out.Spec) + in.Status.DeepCopyInto(&out.Status) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineAccountKey. +func (in *MachineAccountKey) DeepCopy() *MachineAccountKey { + if in == nil { + return nil + } + out := new(MachineAccountKey) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *MachineAccountKey) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *MachineAccountKeyList) DeepCopyInto(out *MachineAccountKeyList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]MachineAccountKey, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineAccountKeyList. +func (in *MachineAccountKeyList) DeepCopy() *MachineAccountKeyList { + if in == nil { + return nil + } + out := new(MachineAccountKeyList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *MachineAccountKeyList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *MachineAccountKeySpec) DeepCopyInto(out *MachineAccountKeySpec) { + *out = *in + if in.ExpirationDate != nil { + in, out := &in.ExpirationDate, &out.ExpirationDate + *out = (*in).DeepCopy() + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineAccountKeySpec. +func (in *MachineAccountKeySpec) DeepCopy() *MachineAccountKeySpec { + if in == nil { + return nil + } + out := new(MachineAccountKeySpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *MachineAccountKeyStatus) DeepCopyInto(out *MachineAccountKeyStatus) { + *out = *in + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]v1.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineAccountKeyStatus. +func (in *MachineAccountKeyStatus) DeepCopy() *MachineAccountKeyStatus { + if in == nil { + return nil + } + out := new(MachineAccountKeyStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Session) DeepCopyInto(out *Session) { *out = *in diff --git a/pkg/apis/identity/v1alpha1/zz_generated.openapi.go b/pkg/apis/identity/v1alpha1/zz_generated.openapi.go index 253a22d2..f216ad83 100644 --- a/pkg/apis/identity/v1alpha1/zz_generated.openapi.go +++ b/pkg/apis/identity/v1alpha1/zz_generated.openapi.go @@ -14,12 +14,200 @@ import ( func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenAPIDefinition { return map[string]common.OpenAPIDefinition{ - "go.miloapis.com/milo/pkg/apis/identity/v1alpha1.Session": schema_pkg_apis_identity_v1alpha1_Session(ref), - "go.miloapis.com/milo/pkg/apis/identity/v1alpha1.SessionList": schema_pkg_apis_identity_v1alpha1_SessionList(ref), - "go.miloapis.com/milo/pkg/apis/identity/v1alpha1.SessionStatus": schema_pkg_apis_identity_v1alpha1_SessionStatus(ref), - "go.miloapis.com/milo/pkg/apis/identity/v1alpha1.UserIdentity": schema_pkg_apis_identity_v1alpha1_UserIdentity(ref), - "go.miloapis.com/milo/pkg/apis/identity/v1alpha1.UserIdentityList": schema_pkg_apis_identity_v1alpha1_UserIdentityList(ref), - "go.miloapis.com/milo/pkg/apis/identity/v1alpha1.UserIdentityStatus": schema_pkg_apis_identity_v1alpha1_UserIdentityStatus(ref), + "go.miloapis.com/milo/pkg/apis/identity/v1alpha1.MachineAccountKey": schema_pkg_apis_identity_v1alpha1_MachineAccountKey(ref), + "go.miloapis.com/milo/pkg/apis/identity/v1alpha1.MachineAccountKeyList": schema_pkg_apis_identity_v1alpha1_MachineAccountKeyList(ref), + "go.miloapis.com/milo/pkg/apis/identity/v1alpha1.MachineAccountKeySpec": schema_pkg_apis_identity_v1alpha1_MachineAccountKeySpec(ref), + "go.miloapis.com/milo/pkg/apis/identity/v1alpha1.MachineAccountKeyStatus": schema_pkg_apis_identity_v1alpha1_MachineAccountKeyStatus(ref), + "go.miloapis.com/milo/pkg/apis/identity/v1alpha1.Session": schema_pkg_apis_identity_v1alpha1_Session(ref), + "go.miloapis.com/milo/pkg/apis/identity/v1alpha1.SessionList": schema_pkg_apis_identity_v1alpha1_SessionList(ref), + "go.miloapis.com/milo/pkg/apis/identity/v1alpha1.SessionStatus": schema_pkg_apis_identity_v1alpha1_SessionStatus(ref), + "go.miloapis.com/milo/pkg/apis/identity/v1alpha1.UserIdentity": schema_pkg_apis_identity_v1alpha1_UserIdentity(ref), + "go.miloapis.com/milo/pkg/apis/identity/v1alpha1.UserIdentityList": schema_pkg_apis_identity_v1alpha1_UserIdentityList(ref), + "go.miloapis.com/milo/pkg/apis/identity/v1alpha1.UserIdentityStatus": schema_pkg_apis_identity_v1alpha1_UserIdentityStatus(ref), + } +} + +func schema_pkg_apis_identity_v1alpha1_MachineAccountKey(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Description: "MachineAccountKey is the Schema for the machineaccountkeys API", + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "kind": { + SchemaProps: spec.SchemaProps{ + Description: "Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds", + Type: []string{"string"}, + Format: "", + }, + }, + "apiVersion": { + SchemaProps: spec.SchemaProps{ + Description: "APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources", + Type: []string{"string"}, + Format: "", + }, + }, + "metadata": { + SchemaProps: spec.SchemaProps{ + Default: map[string]interface{}{}, + Ref: ref("k8s.io/apimachinery/pkg/apis/meta/v1.ObjectMeta"), + }, + }, + "spec": { + SchemaProps: spec.SchemaProps{ + Default: map[string]interface{}{}, + Ref: ref("go.miloapis.com/milo/pkg/apis/identity/v1alpha1.MachineAccountKeySpec"), + }, + }, + "status": { + SchemaProps: spec.SchemaProps{ + Default: map[string]interface{}{}, + Ref: ref("go.miloapis.com/milo/pkg/apis/identity/v1alpha1.MachineAccountKeyStatus"), + }, + }, + }, + }, + }, + Dependencies: []string{ + "go.miloapis.com/milo/pkg/apis/identity/v1alpha1.MachineAccountKeySpec", "go.miloapis.com/milo/pkg/apis/identity/v1alpha1.MachineAccountKeyStatus", "k8s.io/apimachinery/pkg/apis/meta/v1.ObjectMeta"}, + } +} + +func schema_pkg_apis_identity_v1alpha1_MachineAccountKeyList(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Description: "MachineAccountKeyList contains a list of MachineAccountKey", + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "kind": { + SchemaProps: spec.SchemaProps{ + Description: "Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds", + Type: []string{"string"}, + Format: "", + }, + }, + "apiVersion": { + SchemaProps: spec.SchemaProps{ + Description: "APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources", + Type: []string{"string"}, + Format: "", + }, + }, + "metadata": { + SchemaProps: spec.SchemaProps{ + Default: map[string]interface{}{}, + Ref: ref("k8s.io/apimachinery/pkg/apis/meta/v1.ListMeta"), + }, + }, + "items": { + SchemaProps: spec.SchemaProps{ + Type: []string{"array"}, + Items: &spec.SchemaOrArray{ + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: map[string]interface{}{}, + Ref: ref("go.miloapis.com/milo/pkg/apis/identity/v1alpha1.MachineAccountKey"), + }, + }, + }, + }, + }, + }, + Required: []string{"items"}, + }, + }, + Dependencies: []string{ + "go.miloapis.com/milo/pkg/apis/identity/v1alpha1.MachineAccountKey", "k8s.io/apimachinery/pkg/apis/meta/v1.ListMeta"}, + } +} + +func schema_pkg_apis_identity_v1alpha1_MachineAccountKeySpec(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Description: "MachineAccountKeySpec defines the desired state of MachineAccountKey", + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "machineAccountName": { + SchemaProps: spec.SchemaProps{ + Description: "MachineAccountName is the name of the MachineAccount that owns this key.", + Default: "", + Type: []string{"string"}, + Format: "", + }, + }, + "expirationDate": { + SchemaProps: spec.SchemaProps{ + Description: "ExpirationDate is the date and time when the MachineAccountKey will expire. If not specified, the MachineAccountKey will never expire.", + Ref: ref("k8s.io/apimachinery/pkg/apis/meta/v1.Time"), + }, + }, + "publicKey": { + SchemaProps: spec.SchemaProps{ + Description: "PublicKey is the public key of the MachineAccountKey. If not specified, the MachineAccountKey will be created with an auto-generated public key.", + Type: []string{"string"}, + Format: "", + }, + }, + }, + Required: []string{"machineAccountName"}, + }, + }, + Dependencies: []string{ + "k8s.io/apimachinery/pkg/apis/meta/v1.Time"}, + } +} + +func schema_pkg_apis_identity_v1alpha1_MachineAccountKeyStatus(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Description: "MachineAccountKeyStatus defines the observed state of MachineAccountKey", + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "authProviderKeyID": { + SchemaProps: spec.SchemaProps{ + Description: "AuthProviderKeyID is the unique identifier for the key in the auth provider. This field is populated by the controller after the key is created in the auth provider. For example, when using Zitadel, a typical value might be: \"326102453042806786\"", + Type: []string{"string"}, + Format: "", + }, + }, + "privateKey": { + SchemaProps: spec.SchemaProps{ + Description: "PrivateKey contains the PEM-encoded RSA private key generated during resource creation. This field is populated only in the creation response and is never persisted to etcd. Any value present on a GET or LIST response indicates a bug in the server implementation.\n\nNote: private key material will appear in API server audit logs for creation events. This matches the behavior of similar systems (GCP service account keys).", + Type: []string{"string"}, + Format: "", + }, + }, + "conditions": { + VendorExtensible: spec.VendorExtensible{ + Extensions: spec.Extensions{ + "x-kubernetes-list-map-keys": []interface{}{ + "type", + }, + "x-kubernetes-list-type": "map", + }, + }, + SchemaProps: spec.SchemaProps{ + Description: "Conditions provide conditions that represent the current status of the MachineAccountKey.", + Type: []string{"array"}, + Items: &spec.SchemaOrArray{ + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: map[string]interface{}{}, + Ref: ref("k8s.io/apimachinery/pkg/apis/meta/v1.Condition"), + }, + }, + }, + }, + }, + }, + }, + }, + Dependencies: []string{ + "k8s.io/apimachinery/pkg/apis/meta/v1.Condition"}, } } From 28f5da44f7a9a43d73f8bd78679e0a1892e5ed1e Mon Sep 17 00:00:00 2001 From: Jose Szychowski Date: Wed, 25 Mar 2026 14:17:01 -0300 Subject: [PATCH 02/20] feat: implement MachineAccountKey RESTStorage and e2e tests This allows us to have a custom API for the machineAccountKeys, letting provision private keys and public key using Milo as the source of truth. When Milo provision the private key, it won't be stored in the etcd, but will returned in teh update/create response. --- cmd/milo/apiserver/config.go | 4 +- .../samples/iam/v1alpha1/machineaccount.yaml | 8 +- .../identity/v1alpha1/machineaccountkey.yaml | 11 + .../identity/machineaccountkeys/rest.go | 275 +++++++++++ .../identity/machineaccountkeys/rest_test.go | 441 ++++++++++++++++++ .../identity/machineaccountkeys/strategy.go | 193 ++++++++ .../storage/identity/storageprovider.go | 52 ++- pkg/apis/identity/v1alpha1/register.go | 12 +- .../assertions/assert-key-auto-generated.yaml | 7 + .../assertions/assert-key-provided.yaml | 18 + .../chainsaw-test.yaml | 239 ++++++++++ .../resources/key-auto-generate.yaml | 9 + .../resources/key-invalid-expiration.yaml | 8 + .../resources/key-malformed.yaml | 11 + .../resources/key-provided-public.yaml | 17 + .../resources/machine-account.yaml | 7 + .../resources/organization.yaml | 11 + .../resources/project.yaml | 9 + 18 files changed, 1318 insertions(+), 14 deletions(-) create mode 100644 config/samples/identity/v1alpha1/machineaccountkey.yaml create mode 100644 internal/apiserver/identity/machineaccountkeys/rest.go create mode 100644 internal/apiserver/identity/machineaccountkeys/rest_test.go create mode 100644 internal/apiserver/identity/machineaccountkeys/strategy.go create mode 100644 test/identity/machine-account-key-creation/assertions/assert-key-auto-generated.yaml create mode 100644 test/identity/machine-account-key-creation/assertions/assert-key-provided.yaml create mode 100644 test/identity/machine-account-key-creation/chainsaw-test.yaml create mode 100644 test/identity/machine-account-key-creation/resources/key-auto-generate.yaml create mode 100644 test/identity/machine-account-key-creation/resources/key-invalid-expiration.yaml create mode 100644 test/identity/machine-account-key-creation/resources/key-malformed.yaml create mode 100644 test/identity/machine-account-key-creation/resources/key-provided-public.yaml create mode 100644 test/identity/machine-account-key-creation/resources/machine-account.yaml create mode 100644 test/identity/machine-account-key-creation/resources/organization.yaml create mode 100644 test/identity/machine-account-key-creation/resources/project.yaml diff --git a/cmd/milo/apiserver/config.go b/cmd/milo/apiserver/config.go index 544c2f9e..ebc29331 100644 --- a/cmd/milo/apiserver/config.go +++ b/cmd/milo/apiserver/config.go @@ -149,9 +149,7 @@ func (c *CompletedConfig) GenericStorageProviders(discovery discovery.DiscoveryI discoveryrest.StorageProvider{}, } - if utilfeature.DefaultFeatureGate.Enabled(features.Sessions) || utilfeature.DefaultFeatureGate.Enabled(features.UserIdentities) { - providers = append(providers, newIdentityStorageProvider(c)) - } + providers = append(providers, newIdentityStorageProvider(c)) if utilfeature.DefaultFeatureGate.Enabled(features.EventsProxy) { providers = append(providers, newEventsV1StorageProvider(eventsBackend)) diff --git a/config/samples/iam/v1alpha1/machineaccount.yaml b/config/samples/iam/v1alpha1/machineaccount.yaml index 2b778f6e..5b3c3ba8 100644 --- a/config/samples/iam/v1alpha1/machineaccount.yaml +++ b/config/samples/iam/v1alpha1/machineaccount.yaml @@ -4,10 +4,4 @@ metadata: name: example-machine-account namespace: default spec: - ownerRef: - name: "example-project" - uid: "12345678-1234-1234-1234-123456789012" - email: "machine-account@example.com" - description: "A sample machine account for API access" - accessTokenType: "jwt" - active: true \ No newline at end of file + state: Active \ No newline at end of file diff --git a/config/samples/identity/v1alpha1/machineaccountkey.yaml b/config/samples/identity/v1alpha1/machineaccountkey.yaml new file mode 100644 index 00000000..8cf22d99 --- /dev/null +++ b/config/samples/identity/v1alpha1/machineaccountkey.yaml @@ -0,0 +1,11 @@ +apiVersion: identity.miloapis.com/v1alpha1 +kind: MachineAccountKey +metadata: + name: example-machine-account-key-32 + namespace: default +spec: + machineAccountName: example-machine-account + # If not specified, the key will never expire. + # expirationDate: "2026-03-25T10:18:48Z" + # If not specified, an auto-generated public key will be created. + expirationDate: "2027-12-31T23:59:59Z" diff --git a/internal/apiserver/identity/machineaccountkeys/rest.go b/internal/apiserver/identity/machineaccountkeys/rest.go new file mode 100644 index 00000000..9b3f79fc --- /dev/null +++ b/internal/apiserver/identity/machineaccountkeys/rest.go @@ -0,0 +1,275 @@ +package machineaccountkeys + +import ( + "context" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "encoding/pem" + "fmt" + "time" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + generic "k8s.io/apiserver/pkg/registry/generic" + genericregistry "k8s.io/apiserver/pkg/registry/generic/registry" + "k8s.io/apiserver/pkg/registry/rest" + k8smetrics "k8s.io/component-base/metrics" + k8slegacy "k8s.io/component-base/metrics/legacyregistry" + + identityv1alpha1 "go.miloapis.com/milo/pkg/apis/identity/v1alpha1" +) + +// generateRSAKeyPairFunc is a package-level variable to allow test overriding. +var generateRSAKeyPairFunc = generateRSAKeyPair + +// metrics for key generation observability (NFR3) +var ( + keyGenerationDuration = k8smetrics.NewHistogramVec( + &k8smetrics.HistogramOpts{ + Name: "milo_machineaccountkey_generation_duration_seconds", + Help: "Duration of RSA key generation for MachineAccountKey creation", + Buckets: []float64{0.01, 0.025, 0.05, 0.1, 0.25, 0.5, 1.0}, + StabilityLevel: k8smetrics.ALPHA, + }, + []string{"result"}, // "success" | "failure" + ) +) + +func init() { + k8slegacy.MustRegister(keyGenerationDuration) +} + +// REST wraps a genericregistry.Store and intercepts Create to inject RSA key generation. +// It embeds the store to inherit all standard REST operations (Get, List, Watch, Delete, +// ConvertToTable, etc.) and only overrides Create. +type REST struct { + *genericregistry.Store +} + +var _ rest.Scoper = &REST{} +var _ rest.Creater = &REST{} +var _ rest.Updater = &REST{} +var _ rest.Getter = &REST{} +var _ rest.Lister = &REST{} +var _ rest.GracefulDeleter = &REST{} +var _ rest.Watcher = &REST{} +var _ rest.Storage = &REST{} +var _ rest.SingularNameProvider = &REST{} + +// NewREST constructs a REST storage handler backed by etcd via the given RESTOptionsGetter. +// It returns an error if the store cannot be completed (e.g. RESTOptionsGetter is misconfigured). +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, error) { + gr := identityv1alpha1.SchemeGroupVersion.WithResource("machineaccountkeys").GroupResource() + + store := &genericregistry.Store{ + NewFunc: func() runtime.Object { return &identityv1alpha1.MachineAccountKey{} }, + NewListFunc: func() runtime.Object { return &identityv1alpha1.MachineAccountKeyList{} }, + DefaultQualifiedResource: gr, + SingularQualifiedResource: identityv1alpha1.SchemeGroupVersion.WithResource("machineaccountkey").GroupResource(), + CreateStrategy: Strategy, + UpdateStrategy: Strategy, + DeleteStrategy: Strategy, + TableConvertor: rest.NewDefaultTableConvertor(gr), + } + + options := &generic.StoreOptions{ + RESTOptions: optsGetter, + } + + if err := store.CompleteWithOptions(options); err != nil { + return nil, fmt.Errorf("failed to complete machineaccountkeys store: %w", err) + } + + return &REST{Store: store}, nil +} + +func (r *REST) GetSingularName() string { return "machineaccountkey" } + +// Create intercepts the standard create path to optionally generate an RSA key pair +// when spec.publicKey is omitted. The private key is returned in status.privateKey +// of the HTTP response object only — it is never passed to etcd storage. +func (r *REST) Create( + ctx context.Context, + obj runtime.Object, + createValidation rest.ValidateObjectFunc, + options *metav1.CreateOptions, +) (runtime.Object, error) { + key, ok := obj.(*identityv1alpha1.MachineAccountKey) + if !ok { + return nil, apierrors.NewBadRequest(fmt.Sprintf( + "not a MachineAccountKey: %T", obj, + )) + } + + // FR6: validate expiration date is in the future if provided + if key.Spec.ExpirationDate != nil && !key.Spec.ExpirationDate.Time.IsZero() { + if !key.Spec.ExpirationDate.Time.After(time.Now()) { + return nil, apierrors.NewBadRequest("spec.expirationDate must be in the future") + } + } + + // FR7: validate public key format if provided + if key.Spec.PublicKey != "" { + if err := validateRSAPublicKey(key.Spec.PublicKey); err != nil { + return nil, err + } + } + + // FR1: auto-generate RSA 2048-bit key pair when no public key is provided + var privateKeyPEM string + if key.Spec.PublicKey == "" { + start := time.Now() + pubPEM, privPEM, err := generateRSAKeyPairFunc() + elapsed := time.Since(start) + + if err != nil { + keyGenerationDuration.WithLabelValues("failure").Observe(elapsed.Seconds()) + return nil, apierrors.NewInternalError(fmt.Errorf("failed to generate RSA key pair: %w", err)) + } + keyGenerationDuration.WithLabelValues("success").Observe(elapsed.Seconds()) + + key.Spec.PublicKey = pubPEM + privateKeyPEM = privPEM + } + + // Persist to etcd. The strategy's PrepareForCreate ensures Status.PrivateKey + // is cleared before the object reaches storage (defense in depth). + result, err := r.Store.Create(ctx, key, createValidation, options) + if err != nil { + return nil, err + } + + // FR1/FR3: Set the private key on the in-memory response object AFTER the + // etcd write returns. The object stored in etcd never carries the private key. + if privateKeyPEM != "" { + createdKey, ok := result.(*identityv1alpha1.MachineAccountKey) + if ok { + createdKey.Status.PrivateKey = privateKeyPEM + } + } + + return result, nil +} + +// Update intercepts the standard update path to support key rotation. +// It allows updating the publicKey field and optionally auto-generates a new key pair +// if publicKey is set to empty string. The strategy's ValidateUpdate enforces immutability +// of machineAccountName and expirationDate. +func (r *REST) Update( + ctx context.Context, + name string, + objInfo rest.UpdatedObjectInfo, + createValidation rest.ValidateObjectFunc, + updateValidation rest.ValidateObjectUpdateFunc, + forceAllowCreate bool, + options *metav1.UpdateOptions, +) (runtime.Object, bool, error) { + var privateKeyPEM string + + // Wrap objInfo to intercept the update and trigger rotation if publicKey is cleared. + info := &rotationUpdatedObjectInfo{ + UpdatedObjectInfo: objInfo, + update: func(ctx context.Context, obj, old runtime.Object) (runtime.Object, error) { + newKey, ok := obj.(*identityv1alpha1.MachineAccountKey) + if !ok { + return obj, nil + } + + // If the public key is explicitly cleared in the update, generate a new one. + if newKey.Spec.PublicKey == "" { + start := time.Now() + pubPEM, privPEM, err := generateRSAKeyPairFunc() + elapsed := time.Since(start) + + if err != nil { + keyGenerationDuration.WithLabelValues("failure").Observe(elapsed.Seconds()) + return nil, apierrors.NewInternalError(err) + } + keyGenerationDuration.WithLabelValues("success").Observe(elapsed.Seconds()) + + newKey.Spec.PublicKey = pubPEM + privateKeyPEM = privPEM + } + return newKey, nil + }, + } + + result, created, err := r.Store.Update(ctx, name, info, createValidation, updateValidation, forceAllowCreate, options) + if err != nil { + return nil, false, err + } + + // Set the private key on the response object (in-memory only). + if privateKeyPEM != "" { + if updatedKey, ok := result.(*identityv1alpha1.MachineAccountKey); ok { + updatedKey.Status.PrivateKey = privateKeyPEM + } + } + + return result, created, nil +} + +// generateRSAKeyPair generates a 2048-bit RSA key pair and returns +// PEM-encoded public (PKIX) and private (PKCS1) key material. +func generateRSAKeyPair() (publicKeyPEM string, privateKeyPEM string, err error) { + privateKey, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + return "", "", fmt.Errorf("rsa.GenerateKey: %w", err) + } + + pubDER, err := x509.MarshalPKIXPublicKey(&privateKey.PublicKey) + if err != nil { + return "", "", fmt.Errorf("x509.MarshalPKIXPublicKey: %w", err) + } + + pubPEMBlock := pem.EncodeToMemory(&pem.Block{ + Type: "PUBLIC KEY", + Bytes: pubDER, + }) + + privDER := x509.MarshalPKCS1PrivateKey(privateKey) + privPEMBlock := pem.EncodeToMemory(&pem.Block{ + Type: "RSA PRIVATE KEY", + Bytes: privDER, + }) + + return string(pubPEMBlock), string(privPEMBlock), nil +} + +// validateRSAPublicKey returns an API error if the PEM string is not a valid +// PEM-encoded RSA public key. It returns nil when the key is acceptable. +func validateRSAPublicKey(pubKeyPEM string) error { + block, _ := pem.Decode([]byte(pubKeyPEM)) + if block == nil { + return apierrors.NewBadRequest("spec.publicKey must be PEM-encoded") + } + + pub, err := x509.ParsePKIXPublicKey(block.Bytes) + if err != nil { + return apierrors.NewBadRequest(fmt.Sprintf("invalid public key: %v", err)) + } + + if _, ok := pub.(*rsa.PublicKey); !ok { + return apierrors.NewBadRequest("only RSA public keys are supported in v1alpha1") + } + + return nil +} + +// rotationUpdatedObjectInfo wraps rest.UpdatedObjectInfo to allow intercepting +// the resulting object and performing transformations (like key generation) +// before it is passed to the underlying store. +type rotationUpdatedObjectInfo struct { + rest.UpdatedObjectInfo + update func(ctx context.Context, obj, old runtime.Object) (runtime.Object, error) +} + +func (i *rotationUpdatedObjectInfo) UpdatedObject(ctx context.Context, old runtime.Object) (runtime.Object, error) { + newObj, err := i.UpdatedObjectInfo.UpdatedObject(ctx, old) + if err != nil { + return nil, err + } + return i.update(ctx, newObj, old) +} diff --git a/internal/apiserver/identity/machineaccountkeys/rest_test.go b/internal/apiserver/identity/machineaccountkeys/rest_test.go new file mode 100644 index 00000000..d042e90b --- /dev/null +++ b/internal/apiserver/identity/machineaccountkeys/rest_test.go @@ -0,0 +1,441 @@ +package machineaccountkeys + +import ( + "context" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "encoding/pem" + "errors" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + apirequest "k8s.io/apiserver/pkg/endpoints/request" + "k8s.io/apiserver/pkg/registry/rest" + + identityv1alpha1 "go.miloapis.com/milo/pkg/apis/identity/v1alpha1" +) + +// mockStore is a minimal fake underlying store for Create. It captures what +// object was passed to Create so tests can verify the private key was not persisted. +type mockStore struct { + capturedObj runtime.Object + returnObj runtime.Object + returnErr error +} + +func (m *mockStore) Create( + _ context.Context, + obj runtime.Object, + _ rest.ValidateObjectFunc, + _ *metav1.CreateOptions, +) (runtime.Object, error) { + // Deep-copy to capture the state at the time of the store call. + m.capturedObj = obj.DeepCopyObject() + if m.returnErr != nil { + return nil, m.returnErr + } + if m.returnObj != nil { + return m.returnObj, nil + } + return obj, nil +} + +// restWithMockStore builds a REST instance whose embedded Store.Create is replaced +// by a lightweight test double. We bypass NewREST to avoid needing an etcd backend +// in unit tests; only the Create interception logic is under test. +type testREST struct { + store *mockStore +} + +func (r *testREST) Create( + ctx context.Context, + obj runtime.Object, + createValidation rest.ValidateObjectFunc, + options *metav1.CreateOptions, +) (runtime.Object, error) { + key, ok := obj.(*identityv1alpha1.MachineAccountKey) + if !ok { + return nil, apierrors.NewBadRequest("not a MachineAccountKey") + } + + if key.Spec.ExpirationDate != nil && !key.Spec.ExpirationDate.Time.IsZero() { + if !key.Spec.ExpirationDate.Time.After(time.Now()) { + return nil, apierrors.NewBadRequest("spec.expirationDate must be in the future") + } + } + + if key.Spec.PublicKey != "" { + if err := validateRSAPublicKey(key.Spec.PublicKey); err != nil { + return nil, err + } + } + + var privateKeyPEM string + if key.Spec.PublicKey == "" { + pubPEM, privPEM, err := generateRSAKeyPairFunc() + if err != nil { + return nil, apierrors.NewInternalError(err) + } + key.Spec.PublicKey = pubPEM + privateKeyPEM = privPEM + } + + // Simulate strategy PrepareForCreate: clear private key before storage write. + key.Status.PrivateKey = "" + + result, err := r.store.Create(ctx, key, createValidation, options) + if err != nil { + return nil, err + } + + if privateKeyPEM != "" { + createdKey, ok := result.(*identityv1alpha1.MachineAccountKey) + if ok { + createdKey.Status.PrivateKey = privateKeyPEM + } + } + + return result, nil +} + +func newTestREST(store *mockStore) *testREST { + return &testREST{store: store} +} + +func makeKey(name, machineAccountName, publicKey string) *identityv1alpha1.MachineAccountKey { + return &identityv1alpha1.MachineAccountKey{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: "default", + }, + Spec: identityv1alpha1.MachineAccountKeySpec{ + MachineAccountName: machineAccountName, + PublicKey: publicKey, + }, + } +} + +func generateTestRSAPublicKeyPEM(t *testing.T) string { + t.Helper() + priv, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + pubDER, err := x509.MarshalPKIXPublicKey(&priv.PublicKey) + require.NoError(t, err) + return string(pem.EncodeToMemory(&pem.Block{Type: "PUBLIC KEY", Bytes: pubDER})) +} + +func TestCreate_AutoGeneration(t *testing.T) { + store := &mockStore{} + r := newTestREST(store) + + key := makeKey("test-key", "my-machine-account", "") + + ctx := apirequest.WithNamespace(context.Background(), "default") + result, err := r.Create(ctx, key, nil, &metav1.CreateOptions{}) + + require.NoError(t, err) + require.NotNil(t, result) + + createdKey, ok := result.(*identityv1alpha1.MachineAccountKey) + require.True(t, ok) + + // The response must contain a private key. + assert.NotEmpty(t, createdKey.Status.PrivateKey, "private key should appear in the creation response") + assert.Contains(t, createdKey.Status.PrivateKey, "-----BEGIN RSA PRIVATE KEY-----", + "private key should be PEM-encoded PKCS1") + + // The spec must contain a public key. + assert.NotEmpty(t, createdKey.Spec.PublicKey, "public key should be populated in spec") + assert.Contains(t, createdKey.Spec.PublicKey, "-----BEGIN PUBLIC KEY-----") +} + +func TestCreate_AutoGeneration_PrivateKeyAbsentFromStoredObject(t *testing.T) { + store := &mockStore{} + r := newTestREST(store) + + key := makeKey("test-key", "my-machine-account", "") + + ctx := apirequest.WithNamespace(context.Background(), "default") + _, err := r.Create(ctx, key, nil, &metav1.CreateOptions{}) + require.NoError(t, err) + + // The object captured by the store (i.e., what would go to etcd) must have + // no private key — this is the critical security invariant. + stored, ok := store.capturedObj.(*identityv1alpha1.MachineAccountKey) + require.True(t, ok) + assert.Empty(t, stored.Status.PrivateKey, + "private key must NOT be present in the object passed to storage (etcd)") +} + +func TestCreate_ProvidedPublicKey_NoPrivateKeyInResponse(t *testing.T) { + pubKeyPEM := generateTestRSAPublicKeyPEM(t) + + store := &mockStore{} + r := newTestREST(store) + + key := makeKey("test-key", "my-machine-account", pubKeyPEM) + + ctx := apirequest.WithNamespace(context.Background(), "default") + result, err := r.Create(ctx, key, nil, &metav1.CreateOptions{}) + + require.NoError(t, err) + require.NotNil(t, result) + + createdKey, ok := result.(*identityv1alpha1.MachineAccountKey) + require.True(t, ok) + + // No private key should appear when the caller supplies their own public key. + assert.Empty(t, createdKey.Status.PrivateKey, + "status.privateKey should be absent when publicKey is provided") + + // The provided public key should be preserved unchanged. + assert.Equal(t, pubKeyPEM, createdKey.Spec.PublicKey, + "provided public key should be passed through unchanged") +} + +func TestCreate_GenerationFailure_Returns500(t *testing.T) { + // Override the key generation function to simulate a failure. + original := generateRSAKeyPairFunc + defer func() { generateRSAKeyPairFunc = original }() + generateRSAKeyPairFunc = func() (string, string, error) { + return "", "", errors.New("entropy source unavailable") + } + + store := &mockStore{} + r := newTestREST(store) + + key := makeKey("test-key", "my-machine-account", "") + + ctx := apirequest.WithNamespace(context.Background(), "default") + _, err := r.Create(ctx, key, nil, &metav1.CreateOptions{}) + + require.Error(t, err) + statusErr := &apierrors.StatusError{} + require.True(t, errors.As(err, &statusErr)) + assert.Equal(t, int32(500), statusErr.ErrStatus.Code, + "key generation failure should return 500 Internal Server Error") +} + +func TestCreate_StorageFailure_Propagated(t *testing.T) { + expectedErr := errors.New("etcd connection refused") + store := &mockStore{returnErr: expectedErr} + r := newTestREST(store) + + key := makeKey("test-key", "my-machine-account", "") + + ctx := apirequest.WithNamespace(context.Background(), "default") + _, err := r.Create(ctx, key, nil, &metav1.CreateOptions{}) + + require.Error(t, err) + assert.Equal(t, expectedErr, err, "storage errors should propagate unchanged") +} + +func TestCreate_MalformedPublicKey_Returns400(t *testing.T) { + store := &mockStore{} + r := newTestREST(store) + + key := makeKey("test-key", "my-machine-account", "not-a-valid-pem") + + ctx := apirequest.WithNamespace(context.Background(), "default") + _, err := r.Create(ctx, key, nil, &metav1.CreateOptions{}) + + require.Error(t, err) + statusErr := &apierrors.StatusError{} + require.True(t, errors.As(err, &statusErr)) + assert.Equal(t, int32(400), statusErr.ErrStatus.Code) +} + +func TestCreate_CorruptPublicKeyDER_Returns400(t *testing.T) { + // Build a PEM block that decodes correctly but contains truncated DER bytes, + // causing x509.ParsePKIXPublicKey to fail. This exercises the "invalid public key" + // error path in validateRSAPublicKey. + priv, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + pubDER, err := x509.MarshalPKIXPublicKey(&priv.PublicKey) + require.NoError(t, err) + + corruptPEM := string(pem.EncodeToMemory(&pem.Block{ + Type: "PUBLIC KEY", + Bytes: pubDER[:len(pubDER)/2], // truncate to corrupt the DER + })) + + store := &mockStore{} + r := newTestREST(store) + + key := makeKey("test-key", "my-machine-account", corruptPEM) + + ctx := apirequest.WithNamespace(context.Background(), "default") + _, createErr := r.Create(ctx, key, nil, &metav1.CreateOptions{}) + + require.Error(t, createErr) + statusErr := &apierrors.StatusError{} + require.True(t, errors.As(createErr, &statusErr)) + assert.Equal(t, int32(400), statusErr.ErrStatus.Code) +} + +func TestCreate_ExpirationDateInPast_Returns400(t *testing.T) { + store := &mockStore{} + r := newTestREST(store) + + pastTime := metav1.NewTime(time.Now().Add(-24 * time.Hour)) + key := makeKey("test-key", "my-machine-account", "") + key.Spec.ExpirationDate = &pastTime + + ctx := apirequest.WithNamespace(context.Background(), "default") + _, err := r.Create(ctx, key, nil, &metav1.CreateOptions{}) + + require.Error(t, err) + statusErr := &apierrors.StatusError{} + require.True(t, errors.As(err, &statusErr)) + assert.Equal(t, int32(400), statusErr.ErrStatus.Code) + assert.Contains(t, statusErr.ErrStatus.Message, "expirationDate") +} + +func TestCreate_ExpirationDateInFuture_Succeeds(t *testing.T) { + store := &mockStore{} + r := newTestREST(store) + + futureTime := metav1.NewTime(time.Now().Add(24 * time.Hour)) + key := makeKey("test-key", "my-machine-account", "") + key.Spec.ExpirationDate = &futureTime + + ctx := apirequest.WithNamespace(context.Background(), "default") + result, err := r.Create(ctx, key, nil, &metav1.CreateOptions{}) + + require.NoError(t, err) + require.NotNil(t, result) +} + +func TestValidateRSAPublicKey_ValidKey_ReturnsNil(t *testing.T) { + pubKeyPEM := generateTestRSAPublicKeyPEM(t) + err := validateRSAPublicKey(pubKeyPEM) + assert.NoError(t, err) +} + +func TestValidateRSAPublicKey_NotPEM_Returns400(t *testing.T) { + err := validateRSAPublicKey("this is not PEM") + require.Error(t, err) + statusErr := &apierrors.StatusError{} + require.True(t, errors.As(err, &statusErr)) + assert.Equal(t, int32(400), statusErr.ErrStatus.Code) +} + +func TestValidateRSAPublicKey_CorruptDER_Returns400(t *testing.T) { + corruptPEM := string(pem.EncodeToMemory(&pem.Block{ + Type: "PUBLIC KEY", + Bytes: []byte("not valid DER"), + })) + err := validateRSAPublicKey(corruptPEM) + require.Error(t, err) + statusErr := &apierrors.StatusError{} + require.True(t, errors.As(err, &statusErr)) + assert.Equal(t, int32(400), statusErr.ErrStatus.Code) +} + +func TestGenerateRSAKeyPair_ProducesValidKeys(t *testing.T) { + pubPEM, privPEM, err := generateRSAKeyPair() + require.NoError(t, err) + + // Validate public key. + pubBlock, _ := pem.Decode([]byte(pubPEM)) + require.NotNil(t, pubBlock, "public key should be valid PEM") + assert.Equal(t, "PUBLIC KEY", pubBlock.Type) + pub, err := x509.ParsePKIXPublicKey(pubBlock.Bytes) + require.NoError(t, err) + _, ok := pub.(*rsa.PublicKey) + assert.True(t, ok, "public key should be RSA") + + // Validate private key. + privBlock, _ := pem.Decode([]byte(privPEM)) + require.NotNil(t, privBlock, "private key should be valid PEM") + assert.Equal(t, "RSA PRIVATE KEY", privBlock.Type) + priv, err := x509.ParsePKCS1PrivateKey(privBlock.Bytes) + require.NoError(t, err) + assert.Equal(t, 2048, priv.Size()*8, "key should be 2048-bit") +} + +func TestValidateUpdate_BlocksMachineAccountNameChange(t *testing.T) { + oldKey := makeKey("test-key", "original-account", "") + newKey := makeKey("test-key", "changed-account", "") + + errs := Strategy.ValidateUpdate(context.Background(), newKey, oldKey) + + require.Len(t, errs, 1) + assert.Equal(t, "spec.machineAccountName", errs[0].Field) + assert.Contains(t, errs[0].Detail, "immutable") +} + +func TestValidateUpdate_BlocksExpirationDateChange(t *testing.T) { + oldTime := metav1.NewTime(time.Date(2027, 1, 1, 0, 0, 0, 0, time.UTC)) + newTime := metav1.NewTime(time.Date(2028, 1, 1, 0, 0, 0, 0, time.UTC)) + + oldKey := makeKey("test-key", "my-account", "") + oldKey.Spec.ExpirationDate = &oldTime + + newKey := makeKey("test-key", "my-account", "") + newKey.Spec.ExpirationDate = &newTime + + errs := Strategy.ValidateUpdate(context.Background(), newKey, oldKey) + + require.Len(t, errs, 1) + assert.Equal(t, "spec.expirationDate", errs[0].Field) + assert.Contains(t, errs[0].Detail, "immutable") +} + +func TestValidateUpdate_AllowsPublicKeyRotation(t *testing.T) { + oldPubKey := generateTestRSAPublicKeyPEM(t) + newPubKey := generateTestRSAPublicKeyPEM(t) + + oldKey := makeKey("test-key", "my-account", oldPubKey) + newKey := makeKey("test-key", "my-account", newPubKey) + + errs := Strategy.ValidateUpdate(context.Background(), newKey, oldKey) + + require.Len(t, errs, 0, "updating publicKey to a new valid RSA key should be allowed") +} + +func TestValidateUpdate_ValidatesNewPublicKey(t *testing.T) { + oldPubKey := generateTestRSAPublicKeyPEM(t) + + oldKey := makeKey("test-key", "my-account", oldPubKey) + newKey := makeKey("test-key", "my-account", "not-a-valid-pem") + + errs := Strategy.ValidateUpdate(context.Background(), newKey, oldKey) + + require.Len(t, errs, 1) + assert.Equal(t, "spec.publicKey", errs[0].Field) + assert.Contains(t, errs[0].Detail, "must be PEM-encoded") +} + +func TestValidateUpdate_AllowsPublicKeyToEmptyString(t *testing.T) { + oldPubKey := generateTestRSAPublicKeyPEM(t) + + oldKey := makeKey("test-key", "my-account", oldPubKey) + newKey := makeKey("test-key", "my-account", "") + + errs := Strategy.ValidateUpdate(context.Background(), newKey, oldKey) + + require.Len(t, errs, 0, "clearing publicKey should be allowed (handler can auto-generate)") +} + +func TestValidateUpdate_NoChanges_Succeeds(t *testing.T) { + pubKey := generateTestRSAPublicKeyPEM(t) + expTime := metav1.NewTime(time.Date(2027, 1, 1, 0, 0, 0, 0, time.UTC)) + + oldKey := makeKey("test-key", "my-account", pubKey) + oldKey.Spec.ExpirationDate = &expTime + + newKey := makeKey("test-key", "my-account", pubKey) + newKey.Spec.ExpirationDate = &expTime + + errs := Strategy.ValidateUpdate(context.Background(), newKey, oldKey) + + require.Len(t, errs, 0, "no-op update should succeed") +} diff --git a/internal/apiserver/identity/machineaccountkeys/strategy.go b/internal/apiserver/identity/machineaccountkeys/strategy.go new file mode 100644 index 00000000..a4a48b04 --- /dev/null +++ b/internal/apiserver/identity/machineaccountkeys/strategy.go @@ -0,0 +1,193 @@ +package machineaccountkeys + +import ( + "context" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/apiserver/pkg/registry/rest" + "k8s.io/apiserver/pkg/storage/names" + "k8s.io/kubernetes/pkg/api/legacyscheme" + + identityv1alpha1 "go.miloapis.com/milo/pkg/apis/identity/v1alpha1" +) + +// machineAccountKeyStrategy implements rest.RESTCreateStrategy for MachineAccountKey. +type machineAccountKeyStrategy struct { + runtime.ObjectTyper + names.NameGenerator +} + +// Strategy is the singleton strategy instance used by the REST handler's store. +// It uses legacyscheme.Scheme as the ObjectTyper so the store can identify objects +// by their registered GVK. The identity scheme must be installed into legacyscheme.Scheme +// (via identityapi.Install in config.go) before this is used. +var Strategy = machineAccountKeyStrategy{legacyscheme.Scheme, names.SimpleNameGenerator} + +var _ rest.RESTCreateStrategy = machineAccountKeyStrategy{} +var _ rest.RESTUpdateStrategy = machineAccountKeyStrategy{} +var _ rest.RESTDeleteStrategy = machineAccountKeyStrategy{} + +func (machineAccountKeyStrategy) NamespaceScoped() bool { return true } + +// PrepareForCreate clears the PrivateKey field before the object is written to +// etcd. This is the primary defense-in-depth mechanism ensuring the private key +// is never persisted, even if the REST handler incorrectly sets it before calling +// the underlying store. +func (machineAccountKeyStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) { + key, ok := obj.(*identityv1alpha1.MachineAccountKey) + if !ok { + return + } + + // Never persist the private key to etcd. + key.Status.PrivateKey = "" + + // FR8: initialize Ready=Unknown if no conditions are set + if len(key.Status.Conditions) == 0 { + key.Status.Conditions = []metav1.Condition{ + { + Type: "Ready", + Status: metav1.ConditionUnknown, + Reason: "Unknown", + Message: "Waiting for control plane to reconcile", + LastTransitionTime: metav1.NewTime(time.Unix(0, 0).UTC()), + }, + } + } +} + +// PrepareForUpdate clears the PrivateKey field before the object is written to +// etcd (same as PrepareForCreate) to prevent accidental persistence. +func (machineAccountKeyStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) { + key, ok := obj.(*identityv1alpha1.MachineAccountKey) + if !ok { + return + } + + // Never persist the private key to etcd (defense in depth). + key.Status.PrivateKey = "" +} + +// Validate enforces field-level constraints on MachineAccountKey before persistence. +func (machineAccountKeyStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { + key, ok := obj.(*identityv1alpha1.MachineAccountKey) + if !ok { + return field.ErrorList{field.InternalError(field.NewPath(""), nil)} + } + + var errs field.ErrorList + specPath := field.NewPath("spec") + + // FR5: machineAccountName must be non-empty + if key.Spec.MachineAccountName == "" { + errs = append(errs, field.Required(specPath.Child("machineAccountName"), "machineAccountName is required")) + } + + // FR6: expiration date must be in the future if provided + if key.Spec.ExpirationDate != nil && !key.Spec.ExpirationDate.Time.IsZero() { + if !key.Spec.ExpirationDate.Time.After(time.Now()) { + errs = append(errs, field.Invalid( + specPath.Child("expirationDate"), + key.Spec.ExpirationDate, + "expirationDate must be in the future", + )) + } + } + + // FR7: public key must be a valid PEM-encoded RSA public key if provided + if key.Spec.PublicKey != "" { + if err := validateRSAPublicKey(key.Spec.PublicKey); err != nil { + errs = append(errs, field.Invalid( + specPath.Child("publicKey"), + "", + err.Error(), + )) + } + } + + return errs +} + +// ValidateUpdate enforces immutability constraints and validates updates to MachineAccountKey. +// It blocks updates to machineAccountName and expirationDate (immutable fields), +// and validates any new publicKey provided (for key rotation). +func (machineAccountKeyStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { + key, ok := obj.(*identityv1alpha1.MachineAccountKey) + if !ok { + return field.ErrorList{field.InternalError(field.NewPath(""), nil)} + } + + oldKey, ok := old.(*identityv1alpha1.MachineAccountKey) + if !ok { + return field.ErrorList{field.InternalError(field.NewPath(""), nil)} + } + + var errs field.ErrorList + specPath := field.NewPath("spec") + + // Block updates to machineAccountName (immutable) + if key.Spec.MachineAccountName != oldKey.Spec.MachineAccountName { + errs = append(errs, field.Forbidden( + specPath.Child("machineAccountName"), + "machineAccountName is immutable after creation", + )) + } + + // Block updates to expirationDate (immutable) + if !expirationDatesEqual(key.Spec.ExpirationDate, oldKey.Spec.ExpirationDate) { + errs = append(errs, field.Forbidden( + specPath.Child("expirationDate"), + "expirationDate is immutable after creation", + )) + } + + // Validate publicKey if it has changed (allows key rotation) + if key.Spec.PublicKey != oldKey.Spec.PublicKey { + // If new publicKey is non-empty, validate it + if key.Spec.PublicKey != "" { + if err := validateRSAPublicKey(key.Spec.PublicKey); err != nil { + errs = append(errs, field.Invalid( + specPath.Child("publicKey"), + "", + err.Error(), + )) + } + } + // If new publicKey is empty, the REST handler's Update method will handle auto-generation + } + + return errs +} + +// expirationDatesEqual compares two expiration date pointers for equality. +func expirationDatesEqual(a, b *metav1.Time) bool { + if a == nil && b == nil { + return true + } + if a == nil || b == nil { + return false + } + return a.Time.Equal(b.Time) +} + +// Canonicalize normalizes the object. No-op here. +func (machineAccountKeyStrategy) Canonicalize(obj runtime.Object) {} + +// AllowCreateOnUpdate returns false — callers must use POST to create. +func (machineAccountKeyStrategy) AllowCreateOnUpdate() bool { return false } + +// AllowUnconditionalUpdate returns false — updates require a resourceVersion precondition. +func (machineAccountKeyStrategy) AllowUnconditionalUpdate() bool { return false } + +// WarningsOnCreate returns no warnings for MachineAccountKey creation. +func (machineAccountKeyStrategy) WarningsOnCreate(_ context.Context, _ runtime.Object) []string { + return nil +} + +// WarningsOnUpdate returns no warnings for MachineAccountKey updates. +func (machineAccountKeyStrategy) WarningsOnUpdate(_ context.Context, _, _ runtime.Object) []string { + return nil +} diff --git a/internal/apiserver/storage/identity/storageprovider.go b/internal/apiserver/storage/identity/storageprovider.go index 286b97c4..fcd45214 100644 --- a/internal/apiserver/storage/identity/storageprovider.go +++ b/internal/apiserver/storage/identity/storageprovider.go @@ -2,6 +2,9 @@ package identity import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/runtime/serializer" generic "k8s.io/apiserver/pkg/registry/generic" "k8s.io/apiserver/pkg/registry/rest" genericapiserver "k8s.io/apiserver/pkg/server" @@ -9,8 +12,10 @@ import ( "k8s.io/kubernetes/pkg/api/legacyscheme" controlplaneapiserver "k8s.io/kubernetes/pkg/controlplane/apiserver" + machineaccountkeysregistry "go.miloapis.com/milo/internal/apiserver/identity/machineaccountkeys" sessionsregistry "go.miloapis.com/milo/internal/apiserver/identity/sessions" useridentitiesregistry "go.miloapis.com/milo/internal/apiserver/identity/useridentities" + identityapi "go.miloapis.com/milo/pkg/apis/identity" identityv1alpha1 "go.miloapis.com/milo/pkg/apis/identity/v1alpha1" ) @@ -23,7 +28,7 @@ func (p StorageProvider) GroupName() string { return identityv1alpha1.SchemeGrou func (p StorageProvider) NewRESTStorage( _ serverstorage.APIResourceConfigSource, - _ generic.RESTOptionsGetter, + getter generic.RESTOptionsGetter, ) (genericapiserver.APIGroupInfo, error) { apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo( identityv1alpha1.SchemeGroupVersion.Group, @@ -32,9 +37,18 @@ func (p StorageProvider) NewRESTStorage( legacyscheme.Codecs, ) + // Identity types do not have protobuf support, so we wrap the getter to override the + // storage codec with a JSON-only one. Without this, CompleteWithOptions fails with + // "internal type not encodable: object does not implement the protobuf marshalling interface". + machineAccountKeyStorage, err := machineaccountkeysregistry.NewREST(jsonOnlyGetter(getter)) + if err != nil { + return apiGroupInfo, err + } + storage := map[string]rest.Storage{ - "sessions": sessionsregistry.NewREST(p.Sessions), - "useridentities": useridentitiesregistry.NewREST(p.UserIdentities), + "sessions": sessionsregistry.NewREST(p.Sessions), + "useridentities": useridentitiesregistry.NewREST(p.UserIdentities), + "machineaccountkeys": machineAccountKeyStorage, } apiGroupInfo.VersionedResourcesStorageMap = map[string]map[string]rest.Storage{ @@ -45,3 +59,35 @@ func (p StorageProvider) NewRESTStorage( } var _ controlplaneapiserver.RESTStorageProvider = StorageProvider{} + +// jsonOnlyGetter wraps a RESTOptionsGetter to override the storage codec with a +// JSON-only variant for identity resources. This is required because identity types +// do not implement the protobuf marshalling interface, and the default storage factory +// uses protobuf as the preferred encoding format. +type jsonOnlyGetterWrapper struct { + inner generic.RESTOptionsGetter + codec runtime.Codec +} + +func jsonOnlyGetter(inner generic.RESTOptionsGetter) generic.RESTOptionsGetter { + // Build a fresh scheme with only JSON serializers (no protobuf registered). + identityScheme := runtime.NewScheme() + identityapi.Install(identityScheme) + metav1.AddToGroupVersion(identityScheme, schema.GroupVersion{Group: "", Version: "v1"}) + + // serializer.NewCodecFactory registers JSON and YAML only — no protobuf. + identityCodecs := serializer.NewCodecFactory(identityScheme) + jsonCodec := identityCodecs.LegacyCodec(identityv1alpha1.SchemeGroupVersion) + + return &jsonOnlyGetterWrapper{inner: inner, codec: jsonCodec} +} + +func (g *jsonOnlyGetterWrapper) GetRESTOptions(gr schema.GroupResource, example runtime.Object) (generic.RESTOptions, error) { + opts, err := g.inner.GetRESTOptions(gr, example) + if err != nil { + return opts, err + } + // Replace the default codec (which tries protobuf) with a JSON-only one. + opts.StorageConfig.Codec = g.codec + return opts, nil +} diff --git a/pkg/apis/identity/v1alpha1/register.go b/pkg/apis/identity/v1alpha1/register.go index 3c97347a..f838b1eb 100644 --- a/pkg/apis/identity/v1alpha1/register.go +++ b/pkg/apis/identity/v1alpha1/register.go @@ -28,14 +28,24 @@ func Resource(resource string) schema.GroupResource { // Adds the list of known types to Scheme. func addKnownTypes(scheme *runtime.Scheme) error { - scheme.AddKnownTypes(SchemeGroupVersion, + types := []runtime.Object{ &Session{}, &SessionList{}, &UserIdentity{}, &UserIdentityList{}, &MachineAccountKey{}, &MachineAccountKeyList{}, + } + + scheme.AddKnownTypes(SchemeGroupVersion, types...) + scheme.AddKnownTypes(schema.GroupVersion{ + Group: SchemeGroupVersion.Group, + Version: runtime.APIVersionInternal, + }, + &MachineAccountKey{}, + &MachineAccountKeyList{}, ) + metav1.AddToGroupVersion(scheme, SchemeGroupVersion) return nil } diff --git a/test/identity/machine-account-key-creation/assertions/assert-key-auto-generated.yaml b/test/identity/machine-account-key-creation/assertions/assert-key-auto-generated.yaml new file mode 100644 index 00000000..fe9445a9 --- /dev/null +++ b/test/identity/machine-account-key-creation/assertions/assert-key-auto-generated.yaml @@ -0,0 +1,7 @@ +apiVersion: identity.miloapis.com/v1alpha1 +kind: MachineAccountKey +metadata: + name: test-key-auto +spec: + machineAccountName: test-machine-account + expirationDate: "2027-12-31T23:59:59Z" diff --git a/test/identity/machine-account-key-creation/assertions/assert-key-provided.yaml b/test/identity/machine-account-key-creation/assertions/assert-key-provided.yaml new file mode 100644 index 00000000..5ed914e7 --- /dev/null +++ b/test/identity/machine-account-key-creation/assertions/assert-key-provided.yaml @@ -0,0 +1,18 @@ +apiVersion: identity.miloapis.com/v1alpha1 +kind: MachineAccountKey +metadata: + name: test-key-provided +spec: + machineAccountName: test-machine-account + # Should match the provided key + publicKey: | + -----BEGIN PUBLIC KEY----- + MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA1hRF7sbBHvQ8E/vQXPxq + fg8qc3yGR6D75cv6xJwr82QU/twD0eGxVyzRV9Ndhq4DJNdDbKA5rseO36GiGP99 + S5uIUHi+BREdzsuBm+L8K2T5GcE6+5lPmlMjV12qVVJbyWoIEkse/k/zowjSmg5c + Qszc/lDglmWjFfNJG1YtmKqWvUXMNayV5whrpzygMTlHUes7GcGlfQ5gsXKudyYd + HSSeh3bXYfv6IdFj8UoJbFqfIjJNj7Dv4tyF5H7ShQKgSIrXDxzZqtuNLgImmoyE + gFL3FfS1mYO/lNNRctBhN/BkIrAB3bls0JDmJogBBbg9ttyE5QwsRXtV2LIof7Bm + PQIDAQAB + -----END PUBLIC KEY----- + expirationDate: "2027-12-31T23:59:59Z" diff --git a/test/identity/machine-account-key-creation/chainsaw-test.yaml b/test/identity/machine-account-key-creation/chainsaw-test.yaml new file mode 100644 index 00000000..813328bf --- /dev/null +++ b/test/identity/machine-account-key-creation/chainsaw-test.yaml @@ -0,0 +1,239 @@ +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + name: machine-account-key-creation +spec: + description: | + Tests MachineAccountKey creation with automatic RSA key pair generation and validation. + + This test verifies: + - MachineAccountKey can be created without providing a public key (auto-generation) + - Auto-generated RSA public key is stored in spec.publicKey + - Private key is returned in creation response but NOT persisted to etcd + - MachineAccountKey can be created with a provided public key + - No private key is returned when a public key is provided + - Validation rejects expiration dates in the past + - Validation rejects malformed public keys + - Created resources are reconcilable (Ready condition initialized to Unknown) + + steps: + # Setup: create machine account in test namespace + - name: create-machine-account + description: Create a MachineAccount resource for testing keys + try: + - apply: + file: resources/machine-account.yaml + + # Test 1: Auto-generate RSA key pair + - name: test-auto-generate-key + description: Create MachineAccountKey without publicKey → auto-generate RSA pair + try: + # Create the resource and capture the raw response + - script: + timeout: 10s + content: | + # Create the resource and capture the raw response + kubectl apply -f resources/key-auto-generate.yaml -n $NAMESPACE -o json > /tmp/response.json + + # Verify that the response contains a privateKey in status + PRIVATE_KEY=$(jq -r '.status.privateKey // empty' /tmp/response.json) + + if [ -z "$PRIVATE_KEY" ]; then + echo "FAIL: privateKey not found in creation response" + echo "RAW RESPONSE:" + cat /tmp/response.json + exit 1 + fi + + # Verify the privateKey looks like a PEM-encoded RSA key + if ! echo "$PRIVATE_KEY" | grep -q "BEGIN RSA PRIVATE KEY"; then + echo "FAIL: privateKey is not PEM-encoded PKCS1" + exit 1 + fi + + # Verify publicKey was auto-generated in the response + PUBLIC_KEY=$(jq -r '.spec.publicKey // empty' /tmp/response.json) + if [ -z "$PUBLIC_KEY" ]; then + echo "FAIL: publicKey not found in creation response" + exit 1 + fi + + if ! echo "$PUBLIC_KEY" | grep -q "BEGIN PUBLIC KEY"; then + echo "FAIL: publicKey is not PEM-encoded PKIX" + exit 1 + fi + + echo "PASS: privateKey returned in creation response, publicKey auto-generated" + + # Verify the created key exists and is reconcilable + - wait: + apiVersion: identity.miloapis.com/v1alpha1 + kind: MachineAccountKey + name: test-key-auto + timeout: 10s + for: + condition: + name: Ready + value: Unknown + + # Assert the persisted object has correct structure + - assert: + file: assertions/assert-key-auto-generated.yaml + + # Verify privateKey is absent from GET response (not persisted to etcd) + - script: + timeout: 5s + content: | + # Fetch the resource from etcd + kubectl get machineaccountkey test-key-auto -n $NAMESPACE -o json > /tmp/fetched.json + + # Verify privateKey is NOT present + PRIVATE_KEY=$(jq -r '.status.privateKey // empty' /tmp/fetched.json) + + if [ -n "$PRIVATE_KEY" ]; then + echo "FAIL: privateKey should not be stored in etcd" + exit 1 + fi + + echo "PASS: privateKey correctly absent from persisted object" + + # Test 2: Provided public key (no auto-generation) + - name: test-provided-public-key + description: Create MachineAccountKey with publicKey → no auto-generation, no privateKey returned + try: + - script: + timeout: 10s + content: | + # Create the resource with provided public key + kubectl apply -f resources/key-provided-public.yaml -n $NAMESPACE -o json > /tmp/provided_response.json + + # Verify that the response does NOT contain a privateKey + PRIVATE_KEY=$(jq -r '.status.privateKey // empty' /tmp/provided_response.json) + + if [ -n "$PRIVATE_KEY" ]; then + echo "FAIL: privateKey should not be returned when publicKey is provided" + exit 1 + fi + + # Verify the publicKey is present + PUBLIC_KEY=$(jq -r '.spec.publicKey' /tmp/provided_response.json) + if [ -z "$PUBLIC_KEY" ]; then echo "FAIL: publicKey missing"; exit 1; fi + + echo "PASS: no privateKey returned with provided publicKey" + + - wait: + apiVersion: identity.miloapis.com/v1alpha1 + kind: MachineAccountKey + name: test-key-provided + timeout: 10s + for: + condition: + name: Ready + value: Unknown + # Assert the public key matches what was provided + - assert: + file: assertions/assert-key-provided.yaml + + # Test 3: Validation - reject expiration date in past + - name: test-validation-expiration-past + description: Reject MachineAccountKey with expirationDate in the past + try: + - script: + timeout: 10s + content: | + if kubectl apply -f resources/key-invalid-expiration.yaml -n $NAMESPACE 2> /tmp/val_err.json; then + echo "FAIL: resource should have been rejected" + exit 1 + fi + + if grep -q "expirationDate must be in the future" /tmp/val_err.json; then + echo "PASS: expirationDate validation triggered" + else + echo "FAIL: wrong error message" + cat /tmp/val_err.json + exit 1 + fi + + # Test 5: Key Rotation (triggered by clearing publicKey) + - name: test-key-rotation + description: Update existing MachineAccountKey with publicKey="" → rotate and return new privateKey + try: + - script: + timeout: 10s + content: | + # Get current public key for comparison + OLD_PUB=$(kubectl get machineaccountkey test-key-auto -n $NAMESPACE -o jsonpath='{.spec.publicKey}') + + # Patch the resource to trigger rotation (clearing publicKey) + kubectl patch machineaccountkey test-key-auto -n $NAMESPACE --type=merge -p '{"spec":{"publicKey":""}}' -o json > /tmp/rotation_response.json + + # 1. Verify a new privateKey is returned + NEW_PRIV=$(jq -r '.status.privateKey // empty' /tmp/rotation_response.json) + if [ -z "$NEW_PRIV" ]; then echo "FAIL: privateKey not returned on rotation"; exit 1; fi + + # 2. Verify spec.publicKey has changed + NEW_PUB=$(jq -r '.spec.publicKey' /tmp/rotation_response.json) + if [ "$OLD_PUB" == "$NEW_PUB" ]; then echo "FAIL: publicKey did not change after rotation"; exit 1; fi + + # 3. Verify Ready condition is reset to Unknown (signals controller to start) + READY_STATUS=$(jq -r '.status.conditions[] | select(.type=="Ready") | .status' /tmp/rotation_response.json) + if [ "$READY_STATUS" != "Unknown" ]; then echo "FAIL: Ready status not reset to Unknown on rotation"; exit 1; fi + + echo "PASS: machine account key successfully rotated" + + # Test 6: Validation - reject malformed public key + - name: test-validation-malformed-key + description: Reject MachineAccountKey with malformed publicKey + try: + - script: + timeout: 10s + content: | + if kubectl apply -f resources/key-malformed.yaml -n $NAMESPACE 2> /tmp/malformed_err.json; then + echo "FAIL: malformed key should have been rejected" + exit 1 + fi + + if grep -q "publicKey" /tmp/malformed_err.json; then + echo "PASS: malformed key validation triggered" + else + echo "FAIL: wrong error message" + cat /tmp/malformed_err.json + exit 1 + fi + + # Test 7: Verify resource schema and conditions + - name: test-resource-conditions + description: Verify MachineAccountKey initializes with correct conditions + try: + - script: + timeout: 5s + content: | + READY_STATUS=$(kubectl get machineaccountkey test-key-auto -n $NAMESPACE -o json | \ + jq -r '.status.conditions[] | select(.type=="Ready") | .status') + + if [ "$READY_STATUS" != "Unknown" ]; then + echo "FAIL: Expected Ready condition to be Unknown, got $READY_STATUS" + exit 1 + fi + + echo "PASS: conditions are correctly initialized" + + # Cleanup: delete created resources + - name: cleanup + description: Clean up test resources + try: + - delete: + ref: + apiVersion: iam.miloapis.com/v1alpha1 + kind: MachineAccount + name: test-machine-account + - delete: + ref: + apiVersion: identity.miloapis.com/v1alpha1 + kind: MachineAccountKey + name: test-key-auto + - delete: + ref: + apiVersion: identity.miloapis.com/v1alpha1 + kind: MachineAccountKey + name: test-key-provided diff --git a/test/identity/machine-account-key-creation/resources/key-auto-generate.yaml b/test/identity/machine-account-key-creation/resources/key-auto-generate.yaml new file mode 100644 index 00000000..81e59539 --- /dev/null +++ b/test/identity/machine-account-key-creation/resources/key-auto-generate.yaml @@ -0,0 +1,9 @@ +apiVersion: identity.miloapis.com/v1alpha1 +kind: MachineAccountKey +metadata: + name: test-key-auto + +spec: + machineAccountName: test-machine-account + expirationDate: "2027-12-31T23:59:59Z" + publicKey: "" diff --git a/test/identity/machine-account-key-creation/resources/key-invalid-expiration.yaml b/test/identity/machine-account-key-creation/resources/key-invalid-expiration.yaml new file mode 100644 index 00000000..10618fbb --- /dev/null +++ b/test/identity/machine-account-key-creation/resources/key-invalid-expiration.yaml @@ -0,0 +1,8 @@ +apiVersion: identity.miloapis.com/v1alpha1 +kind: MachineAccountKey +metadata: + name: test-key-invalid-exp + +spec: + machineAccountName: test-machine-account + expirationDate: "2020-01-01T00:00:00Z" diff --git a/test/identity/machine-account-key-creation/resources/key-malformed.yaml b/test/identity/machine-account-key-creation/resources/key-malformed.yaml new file mode 100644 index 00000000..109eae90 --- /dev/null +++ b/test/identity/machine-account-key-creation/resources/key-malformed.yaml @@ -0,0 +1,11 @@ +apiVersion: identity.miloapis.com/v1alpha1 +kind: MachineAccountKey +metadata: + name: test-key-malformed + +spec: + machineAccountName: example-machine-account + publicKey: | + -----BEGIN PUBLIC KEY----- + THIS-IS-NOT-A-KEY + -----END PUBLIC KEY----- diff --git a/test/identity/machine-account-key-creation/resources/key-provided-public.yaml b/test/identity/machine-account-key-creation/resources/key-provided-public.yaml new file mode 100644 index 00000000..12e22f32 --- /dev/null +++ b/test/identity/machine-account-key-creation/resources/key-provided-public.yaml @@ -0,0 +1,17 @@ +apiVersion: identity.miloapis.com/v1alpha1 +kind: MachineAccountKey +metadata: + name: test-key-provided +spec: + machineAccountName: test-machine-account + publicKey: | + -----BEGIN PUBLIC KEY----- + MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA1hRF7sbBHvQ8E/vQXPxq + fg8qc3yGR6D75cv6xJwr82QU/twD0eGxVyzRV9Ndhq4DJNdDbKA5rseO36GiGP99 + S5uIUHi+BREdzsuBm+L8K2T5GcE6+5lPmlMjV12qVVJbyWoIEkse/k/zowjSmg5c + Qszc/lDglmWjFfNJG1YtmKqWvUXMNayV5whrpzygMTlHUes7GcGlfQ5gsXKudyYd + HSSeh3bXYfv6IdFj8UoJbFqfIjJNj7Dv4tyF5H7ShQKgSIrXDxzZqtuNLgImmoyE + gFL3FfS1mYO/lNNRctBhN/BkIrAB3bls0JDmJogBBbg9ttyE5QwsRXtV2LIof7Bm + PQIDAQAB + -----END PUBLIC KEY----- + expirationDate: "2027-12-31T23:59:59Z" diff --git a/test/identity/machine-account-key-creation/resources/machine-account.yaml b/test/identity/machine-account-key-creation/resources/machine-account.yaml new file mode 100644 index 00000000..ec68c7f1 --- /dev/null +++ b/test/identity/machine-account-key-creation/resources/machine-account.yaml @@ -0,0 +1,7 @@ +apiVersion: iam.miloapis.com/v1alpha1 +kind: MachineAccount +metadata: + name: test-machine-account + +spec: + state: Active diff --git a/test/identity/machine-account-key-creation/resources/organization.yaml b/test/identity/machine-account-key-creation/resources/organization.yaml new file mode 100644 index 00000000..b8e93387 --- /dev/null +++ b/test/identity/machine-account-key-creation/resources/organization.yaml @@ -0,0 +1,11 @@ +apiVersion: resourcemanager.miloapis.com/v1alpha1 +kind: Organization +metadata: + name: mak-test-org +spec: + type: Standard +--- +apiVersion: v1 +kind: Namespace +metadata: + name: organization-mak-test-org diff --git a/test/identity/machine-account-key-creation/resources/project.yaml b/test/identity/machine-account-key-creation/resources/project.yaml new file mode 100644 index 00000000..69a12f18 --- /dev/null +++ b/test/identity/machine-account-key-creation/resources/project.yaml @@ -0,0 +1,9 @@ +apiVersion: resourcemanager.miloapis.com/v1alpha1 +kind: Project +metadata: + name: mak-test-project + namespace: organization-mak-test-org +spec: + ownerRef: + kind: Organization + name: mak-test-org From 1092e155412237794d282e6995b140dcaddec4ec Mon Sep 17 00:00:00 2001 From: Jose Szychowski Date: Wed, 25 Mar 2026 14:17:55 -0300 Subject: [PATCH 03/20] feat: implement protected resources and roles for machineAccount and machineAccountKey --- .../iam/kustomization.yaml | 1 + .../iam/machineaccount.yaml | 21 +++++++++++++++++++ .../identity/kustomization.yaml | 1 + .../identity/machineaccountkey.yaml | 21 +++++++++++++++++++ config/roles/iam-editor.yaml | 4 ++++ config/roles/iam-machine-accounts-admin.yaml | 11 ++++++++++ config/roles/iam-machine-accounts-editor.yaml | 16 ++++++++++++++ config/roles/iam-machine-accounts-viewer.yaml | 13 ++++++++++++ config/roles/iam-viewer.yaml | 3 +++ .../identity-machine-account-keys-admin.yaml | 11 ++++++++++ .../identity-machine-account-keys-editor.yaml | 16 ++++++++++++++ .../identity-machine-account-keys-viewer.yaml | 13 ++++++++++++ config/roles/kustomization.yaml | 6 ++++++ 13 files changed, 137 insertions(+) create mode 100644 config/protected-resources/iam/machineaccount.yaml create mode 100644 config/protected-resources/identity/machineaccountkey.yaml create mode 100644 config/roles/iam-machine-accounts-admin.yaml create mode 100644 config/roles/iam-machine-accounts-editor.yaml create mode 100644 config/roles/iam-machine-accounts-viewer.yaml create mode 100644 config/roles/identity-machine-account-keys-admin.yaml create mode 100644 config/roles/identity-machine-account-keys-editor.yaml create mode 100644 config/roles/identity-machine-account-keys-viewer.yaml diff --git a/config/protected-resources/iam/kustomization.yaml b/config/protected-resources/iam/kustomization.yaml index 86804e39..69b40ea8 100644 --- a/config/protected-resources/iam/kustomization.yaml +++ b/config/protected-resources/iam/kustomization.yaml @@ -14,4 +14,5 @@ resources: - platformaccessapproval.yaml - platformaccessrejection.yaml - platforminvitation.yaml + - machineaccount.yaml diff --git a/config/protected-resources/iam/machineaccount.yaml b/config/protected-resources/iam/machineaccount.yaml new file mode 100644 index 00000000..154d6c11 --- /dev/null +++ b/config/protected-resources/iam/machineaccount.yaml @@ -0,0 +1,21 @@ +apiVersion: iam.miloapis.com/v1alpha1 +kind: ProtectedResource +metadata: + name: iam.miloapis.com-machineaccount +spec: + serviceRef: + name: "iam.miloapis.com" + kind: MachineAccount + plural: machineaccounts + singular: machineaccount + permissions: + - list + - get + - create + - update + - delete + - patch + - watch + parentResources: + - apiGroup: resourcemanager.miloapis.com + kind: Project diff --git a/config/protected-resources/identity/kustomization.yaml b/config/protected-resources/identity/kustomization.yaml index 88c6f0ac..600a9041 100644 --- a/config/protected-resources/identity/kustomization.yaml +++ b/config/protected-resources/identity/kustomization.yaml @@ -4,3 +4,4 @@ kind: Kustomization resources: - session.yaml - useridentity.yaml + - machineaccountkey.yaml diff --git a/config/protected-resources/identity/machineaccountkey.yaml b/config/protected-resources/identity/machineaccountkey.yaml new file mode 100644 index 00000000..72baf7a3 --- /dev/null +++ b/config/protected-resources/identity/machineaccountkey.yaml @@ -0,0 +1,21 @@ +apiVersion: iam.miloapis.com/v1alpha1 +kind: ProtectedResource +metadata: + name: identity.miloapis.com-machineaccountkey +spec: + serviceRef: + name: "identity.miloapis.com" + kind: MachineAccountKey + plural: machineaccountkeys + singular: machineaccountkey + permissions: + - list + - get + - create + - update + - delete + - patch + - watch + parentResources: + - apiGroup: resourcemanager.miloapis.com + kind: Project diff --git a/config/roles/iam-editor.yaml b/config/roles/iam-editor.yaml index f1c084cf..c9488fff 100644 --- a/config/roles/iam-editor.yaml +++ b/config/roles/iam-editor.yaml @@ -22,6 +22,10 @@ spec: - iam.miloapis.com/userinvitations.update - iam.miloapis.com/userinvitations.patch - iam.miloapis.com/userinvitations.delete + - iam.miloapis.com/machineaccounts.create + - iam.miloapis.com/machineaccounts.update + - iam.miloapis.com/machineaccounts.patch + - iam.miloapis.com/machineaccounts.delete - iam.miloapis.com/policybindings.create - iam.miloapis.com/policybindings.update - iam.miloapis.com/policybindings.patch diff --git a/config/roles/iam-machine-accounts-admin.yaml b/config/roles/iam-machine-accounts-admin.yaml new file mode 100644 index 00000000..78ea9fb8 --- /dev/null +++ b/config/roles/iam-machine-accounts-admin.yaml @@ -0,0 +1,11 @@ +apiVersion: iam.miloapis.com/v1alpha1 +kind: Role +metadata: + name: iam-machine-accounts-admin + annotations: + kubernetes.io/display-name: IAM Machine Accounts Admin + kubernetes.io/description: "Full access to machine accounts." +spec: + launchStage: Beta + inheritedRoles: + - name: iam-machine-accounts-editor diff --git a/config/roles/iam-machine-accounts-editor.yaml b/config/roles/iam-machine-accounts-editor.yaml new file mode 100644 index 00000000..405d0015 --- /dev/null +++ b/config/roles/iam-machine-accounts-editor.yaml @@ -0,0 +1,16 @@ +apiVersion: iam.miloapis.com/v1alpha1 +kind: Role +metadata: + name: iam-machine-accounts-editor + annotations: + kubernetes.io/display-name: IAM Machine Accounts Editor + kubernetes.io/description: "Allows editing machine accounts." +spec: + launchStage: Beta + inheritedRoles: + - name: iam-machine-accounts-viewer + includedPermissions: + - iam.miloapis.com/machineaccounts.create + - iam.miloapis.com/machineaccounts.update + - iam.miloapis.com/machineaccounts.patch + - iam.miloapis.com/machineaccounts.delete diff --git a/config/roles/iam-machine-accounts-viewer.yaml b/config/roles/iam-machine-accounts-viewer.yaml new file mode 100644 index 00000000..e79a8228 --- /dev/null +++ b/config/roles/iam-machine-accounts-viewer.yaml @@ -0,0 +1,13 @@ +apiVersion: iam.miloapis.com/v1alpha1 +kind: Role +metadata: + name: iam-machine-accounts-viewer + annotations: + kubernetes.io/display-name: IAM Machine Accounts Viewer + kubernetes.io/description: "Allows viewing machine accounts." +spec: + launchStage: Beta + includedPermissions: + - iam.miloapis.com/machineaccounts.get + - iam.miloapis.com/machineaccounts.list + - iam.miloapis.com/machineaccounts.watch diff --git a/config/roles/iam-viewer.yaml b/config/roles/iam-viewer.yaml index 99279153..45d73730 100644 --- a/config/roles/iam-viewer.yaml +++ b/config/roles/iam-viewer.yaml @@ -24,6 +24,9 @@ spec: - iam.miloapis.com/userinvitations.get - iam.miloapis.com/userinvitations.list - iam.miloapis.com/userinvitations.watch + - iam.miloapis.com/machineaccounts.get + - iam.miloapis.com/machineaccounts.list + - iam.miloapis.com/machineaccounts.watch - iam.miloapis.com/protectedresources.get - iam.miloapis.com/protectedresources.list - iam.miloapis.com/protectedresources.watch diff --git a/config/roles/identity-machine-account-keys-admin.yaml b/config/roles/identity-machine-account-keys-admin.yaml new file mode 100644 index 00000000..7367ca51 --- /dev/null +++ b/config/roles/identity-machine-account-keys-admin.yaml @@ -0,0 +1,11 @@ +apiVersion: iam.miloapis.com/v1alpha1 +kind: Role +metadata: + name: identity-machine-account-keys-admin + annotations: + kubernetes.io/display-name: Identity Machine Account Keys Admin + kubernetes.io/description: "Full access to machine account keys." +spec: + launchStage: Beta + inheritedRoles: + - name: identity-machine-account-keys-editor diff --git a/config/roles/identity-machine-account-keys-editor.yaml b/config/roles/identity-machine-account-keys-editor.yaml new file mode 100644 index 00000000..ff0447c4 --- /dev/null +++ b/config/roles/identity-machine-account-keys-editor.yaml @@ -0,0 +1,16 @@ +apiVersion: iam.miloapis.com/v1alpha1 +kind: Role +metadata: + name: identity-machine-account-keys-editor + annotations: + kubernetes.io/display-name: Identity Machine Account Keys Editor + kubernetes.io/description: "Allows editing machine account keys." +spec: + launchStage: Beta + inheritedRoles: + - name: identity-machine-account-keys-viewer + includedPermissions: + - identity.miloapis.com/machineaccountkeys.create + - identity.miloapis.com/machineaccountkeys.update + - identity.miloapis.com/machineaccountkeys.patch + - identity.miloapis.com/machineaccountkeys.delete diff --git a/config/roles/identity-machine-account-keys-viewer.yaml b/config/roles/identity-machine-account-keys-viewer.yaml new file mode 100644 index 00000000..3b21ef6c --- /dev/null +++ b/config/roles/identity-machine-account-keys-viewer.yaml @@ -0,0 +1,13 @@ +apiVersion: iam.miloapis.com/v1alpha1 +kind: Role +metadata: + name: identity-machine-account-keys-viewer + annotations: + kubernetes.io/display-name: Identity Machine Account Keys Viewer + kubernetes.io/description: "Allows viewing machine account keys." +spec: + launchStage: Beta + includedPermissions: + - identity.miloapis.com/machineaccountkeys.get + - identity.miloapis.com/machineaccountkeys.list + - identity.miloapis.com/machineaccountkeys.watch diff --git a/config/roles/kustomization.yaml b/config/roles/kustomization.yaml index 816d7f8b..73b43978 100644 --- a/config/roles/kustomization.yaml +++ b/config/roles/kustomization.yaml @@ -71,3 +71,9 @@ resources: - notes-creator-editor.yaml - notes-admin.yaml - identity-user-session-viewer.yaml + - iam-machine-accounts-viewer.yaml + - iam-machine-accounts-editor.yaml + - iam-machine-accounts-admin.yaml + - identity-machine-account-keys-viewer.yaml + - identity-machine-account-keys-editor.yaml + - identity-machine-account-keys-admin.yaml From c95dd4422f8f85b61c0b5ec9d3234ff46d0a767d Mon Sep 17 00:00:00 2001 From: Jose Szychowski Date: Wed, 25 Mar 2026 14:49:59 -0300 Subject: [PATCH 04/20] fix: add missing Kustomize component to generate identity resource metrics ConfigMap --- config/resources-metrics/identity/kustomization.yaml | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 config/resources-metrics/identity/kustomization.yaml diff --git a/config/resources-metrics/identity/kustomization.yaml b/config/resources-metrics/identity/kustomization.yaml new file mode 100644 index 00000000..de9c25ee --- /dev/null +++ b/config/resources-metrics/identity/kustomization.yaml @@ -0,0 +1,10 @@ +apiVersion: kustomize.config.k8s.io/v1alpha1 +kind: Component + +configMapGenerator: + - name: milo-identity-resource-metrics + files: + - machine_account_keys.yaml + options: + labels: + telemetry.datumapis.com/core-resource-metrics-config: "true" From 95150cd927e5d60e65b153690aa756d9cef6ee8e Mon Sep 17 00:00:00 2001 From: Jose Szychowski Date: Wed, 25 Mar 2026 17:24:54 -0300 Subject: [PATCH 05/20] refactor: Enforce MachineAccountKey spec immutability and remove custom update logic for key rotation. --- .../identity/machineaccountkeys/rest.go | 75 +------------------ .../identity/machineaccountkeys/rest_test.go | 44 +++-------- .../identity/machineaccountkeys/strategy.go | 47 ++---------- .../chainsaw-test.yaml | 58 +++++--------- 4 files changed, 35 insertions(+), 189 deletions(-) diff --git a/internal/apiserver/identity/machineaccountkeys/rest.go b/internal/apiserver/identity/machineaccountkeys/rest.go index 9b3f79fc..7a871fd8 100644 --- a/internal/apiserver/identity/machineaccountkeys/rest.go +++ b/internal/apiserver/identity/machineaccountkeys/rest.go @@ -153,63 +153,8 @@ func (r *REST) Create( return result, nil } -// Update intercepts the standard update path to support key rotation. -// It allows updating the publicKey field and optionally auto-generates a new key pair -// if publicKey is set to empty string. The strategy's ValidateUpdate enforces immutability -// of machineAccountName and expirationDate. -func (r *REST) Update( - ctx context.Context, - name string, - objInfo rest.UpdatedObjectInfo, - createValidation rest.ValidateObjectFunc, - updateValidation rest.ValidateObjectUpdateFunc, - forceAllowCreate bool, - options *metav1.UpdateOptions, -) (runtime.Object, bool, error) { - var privateKeyPEM string - - // Wrap objInfo to intercept the update and trigger rotation if publicKey is cleared. - info := &rotationUpdatedObjectInfo{ - UpdatedObjectInfo: objInfo, - update: func(ctx context.Context, obj, old runtime.Object) (runtime.Object, error) { - newKey, ok := obj.(*identityv1alpha1.MachineAccountKey) - if !ok { - return obj, nil - } - - // If the public key is explicitly cleared in the update, generate a new one. - if newKey.Spec.PublicKey == "" { - start := time.Now() - pubPEM, privPEM, err := generateRSAKeyPairFunc() - elapsed := time.Since(start) - - if err != nil { - keyGenerationDuration.WithLabelValues("failure").Observe(elapsed.Seconds()) - return nil, apierrors.NewInternalError(err) - } - keyGenerationDuration.WithLabelValues("success").Observe(elapsed.Seconds()) - - newKey.Spec.PublicKey = pubPEM - privateKeyPEM = privPEM - } - return newKey, nil - }, - } - - result, created, err := r.Store.Update(ctx, name, info, createValidation, updateValidation, forceAllowCreate, options) - if err != nil { - return nil, false, err - } - - // Set the private key on the response object (in-memory only). - if privateKeyPEM != "" { - if updatedKey, ok := result.(*identityv1alpha1.MachineAccountKey); ok { - updatedKey.Status.PrivateKey = privateKeyPEM - } - } - - return result, created, nil -} +// Update is omitted here as it's not needed. The embedded genericregistry.Store.Update will be used. +// The strategy's ValidateUpdate will still be called to enforce Spec immutability. // generateRSAKeyPair generates a 2048-bit RSA key pair and returns // PEM-encoded public (PKIX) and private (PKCS1) key material. @@ -257,19 +202,3 @@ func validateRSAPublicKey(pubKeyPEM string) error { return nil } - -// rotationUpdatedObjectInfo wraps rest.UpdatedObjectInfo to allow intercepting -// the resulting object and performing transformations (like key generation) -// before it is passed to the underlying store. -type rotationUpdatedObjectInfo struct { - rest.UpdatedObjectInfo - update func(ctx context.Context, obj, old runtime.Object) (runtime.Object, error) -} - -func (i *rotationUpdatedObjectInfo) UpdatedObject(ctx context.Context, old runtime.Object) (runtime.Object, error) { - newObj, err := i.UpdatedObjectInfo.UpdatedObject(ctx, old) - if err != nil { - return nil, err - } - return i.update(ctx, newObj, old) -} diff --git a/internal/apiserver/identity/machineaccountkeys/rest_test.go b/internal/apiserver/identity/machineaccountkeys/rest_test.go index d042e90b..b807feb7 100644 --- a/internal/apiserver/identity/machineaccountkeys/rest_test.go +++ b/internal/apiserver/identity/machineaccountkeys/rest_test.go @@ -361,35 +361,18 @@ func TestGenerateRSAKeyPair_ProducesValidKeys(t *testing.T) { assert.Equal(t, 2048, priv.Size()*8, "key should be 2048-bit") } -func TestValidateUpdate_BlocksMachineAccountNameChange(t *testing.T) { +func TestValidateUpdate_BlocksSpecChange(t *testing.T) { oldKey := makeKey("test-key", "original-account", "") newKey := makeKey("test-key", "changed-account", "") errs := Strategy.ValidateUpdate(context.Background(), newKey, oldKey) require.Len(t, errs, 1) - assert.Equal(t, "spec.machineAccountName", errs[0].Field) + assert.Equal(t, "spec", errs[0].Field) assert.Contains(t, errs[0].Detail, "immutable") } -func TestValidateUpdate_BlocksExpirationDateChange(t *testing.T) { - oldTime := metav1.NewTime(time.Date(2027, 1, 1, 0, 0, 0, 0, time.UTC)) - newTime := metav1.NewTime(time.Date(2028, 1, 1, 0, 0, 0, 0, time.UTC)) - - oldKey := makeKey("test-key", "my-account", "") - oldKey.Spec.ExpirationDate = &oldTime - - newKey := makeKey("test-key", "my-account", "") - newKey.Spec.ExpirationDate = &newTime - - errs := Strategy.ValidateUpdate(context.Background(), newKey, oldKey) - - require.Len(t, errs, 1) - assert.Equal(t, "spec.expirationDate", errs[0].Field) - assert.Contains(t, errs[0].Detail, "immutable") -} - -func TestValidateUpdate_AllowsPublicKeyRotation(t *testing.T) { +func TestValidateUpdate_BlocksPublicKeyRotation(t *testing.T) { oldPubKey := generateTestRSAPublicKeyPEM(t) newPubKey := generateTestRSAPublicKeyPEM(t) @@ -398,23 +381,12 @@ func TestValidateUpdate_AllowsPublicKeyRotation(t *testing.T) { errs := Strategy.ValidateUpdate(context.Background(), newKey, oldKey) - require.Len(t, errs, 0, "updating publicKey to a new valid RSA key should be allowed") -} - -func TestValidateUpdate_ValidatesNewPublicKey(t *testing.T) { - oldPubKey := generateTestRSAPublicKeyPEM(t) - - oldKey := makeKey("test-key", "my-account", oldPubKey) - newKey := makeKey("test-key", "my-account", "not-a-valid-pem") - - errs := Strategy.ValidateUpdate(context.Background(), newKey, oldKey) - require.Len(t, errs, 1) - assert.Equal(t, "spec.publicKey", errs[0].Field) - assert.Contains(t, errs[0].Detail, "must be PEM-encoded") + assert.Equal(t, "spec", errs[0].Field) + assert.Contains(t, errs[0].Detail, "immutable") } -func TestValidateUpdate_AllowsPublicKeyToEmptyString(t *testing.T) { +func TestValidateUpdate_BlocksPublicKeyToEmptyString(t *testing.T) { oldPubKey := generateTestRSAPublicKeyPEM(t) oldKey := makeKey("test-key", "my-account", oldPubKey) @@ -422,7 +394,9 @@ func TestValidateUpdate_AllowsPublicKeyToEmptyString(t *testing.T) { errs := Strategy.ValidateUpdate(context.Background(), newKey, oldKey) - require.Len(t, errs, 0, "clearing publicKey should be allowed (handler can auto-generate)") + require.Len(t, errs, 1) + assert.Equal(t, "spec", errs[0].Field) + assert.Contains(t, errs[0].Detail, "immutable") } func TestValidateUpdate_NoChanges_Succeeds(t *testing.T) { diff --git a/internal/apiserver/identity/machineaccountkeys/strategy.go b/internal/apiserver/identity/machineaccountkeys/strategy.go index a4a48b04..94174659 100644 --- a/internal/apiserver/identity/machineaccountkeys/strategy.go +++ b/internal/apiserver/identity/machineaccountkeys/strategy.go @@ -4,6 +4,7 @@ import ( "context" "time" + apiequality "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" @@ -112,8 +113,7 @@ func (machineAccountKeyStrategy) Validate(ctx context.Context, obj runtime.Objec } // ValidateUpdate enforces immutability constraints and validates updates to MachineAccountKey. -// It blocks updates to machineAccountName and expirationDate (immutable fields), -// and validates any new publicKey provided (for key rotation). +// It blocks any updates to the Spec after creation. func (machineAccountKeyStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { key, ok := obj.(*identityv1alpha1.MachineAccountKey) if !ok { @@ -126,53 +126,18 @@ func (machineAccountKeyStrategy) ValidateUpdate(ctx context.Context, obj, old ru } var errs field.ErrorList - specPath := field.NewPath("spec") - - // Block updates to machineAccountName (immutable) - if key.Spec.MachineAccountName != oldKey.Spec.MachineAccountName { - errs = append(errs, field.Forbidden( - specPath.Child("machineAccountName"), - "machineAccountName is immutable after creation", - )) - } - // Block updates to expirationDate (immutable) - if !expirationDatesEqual(key.Spec.ExpirationDate, oldKey.Spec.ExpirationDate) { + // Block all updates to Spec after creation + if !apiequality.Semantic.DeepEqual(key.Spec, oldKey.Spec) { errs = append(errs, field.Forbidden( - specPath.Child("expirationDate"), - "expirationDate is immutable after creation", + field.NewPath("spec"), + "spec is immutable after creation", )) } - // Validate publicKey if it has changed (allows key rotation) - if key.Spec.PublicKey != oldKey.Spec.PublicKey { - // If new publicKey is non-empty, validate it - if key.Spec.PublicKey != "" { - if err := validateRSAPublicKey(key.Spec.PublicKey); err != nil { - errs = append(errs, field.Invalid( - specPath.Child("publicKey"), - "", - err.Error(), - )) - } - } - // If new publicKey is empty, the REST handler's Update method will handle auto-generation - } - return errs } -// expirationDatesEqual compares two expiration date pointers for equality. -func expirationDatesEqual(a, b *metav1.Time) bool { - if a == nil && b == nil { - return true - } - if a == nil || b == nil { - return false - } - return a.Time.Equal(b.Time) -} - // Canonicalize normalizes the object. No-op here. func (machineAccountKeyStrategy) Canonicalize(obj runtime.Object) {} diff --git a/test/identity/machine-account-key-creation/chainsaw-test.yaml b/test/identity/machine-account-key-creation/chainsaw-test.yaml index 813328bf..af79fffa 100644 --- a/test/identity/machine-account-key-creation/chainsaw-test.yaml +++ b/test/identity/machine-account-key-creation/chainsaw-test.yaml @@ -14,6 +14,7 @@ spec: - No private key is returned when a public key is provided - Validation rejects expiration dates in the past - Validation rejects malformed public keys + - Validation rejects any updates to spec after creation (immutability) - Created resources are reconcilable (Ready condition initialized to Unknown) steps: @@ -154,32 +155,26 @@ spec: exit 1 fi - # Test 5: Key Rotation (triggered by clearing publicKey) - - name: test-key-rotation - description: Update existing MachineAccountKey with publicKey="" → rotate and return new privateKey + # Test 5: Immutability - reject any spec update + - name: test-spec-immutability + description: Reject any update to MachineAccountKey.spec after creation try: - script: timeout: 10s content: | - # Get current public key for comparison - OLD_PUB=$(kubectl get machineaccountkey test-key-auto -n $NAMESPACE -o jsonpath='{.spec.publicKey}') - - # Patch the resource to trigger rotation (clearing publicKey) - kubectl patch machineaccountkey test-key-auto -n $NAMESPACE --type=merge -p '{"spec":{"publicKey":""}}' -o json > /tmp/rotation_response.json - - # 1. Verify a new privateKey is returned - NEW_PRIV=$(jq -r '.status.privateKey // empty' /tmp/rotation_response.json) - if [ -z "$NEW_PRIV" ]; then echo "FAIL: privateKey not returned on rotation"; exit 1; fi - - # 2. Verify spec.publicKey has changed - NEW_PUB=$(jq -r '.spec.publicKey' /tmp/rotation_response.json) - if [ "$OLD_PUB" == "$NEW_PUB" ]; then echo "FAIL: publicKey did not change after rotation"; exit 1; fi - - # 3. Verify Ready condition is reset to Unknown (signals controller to start) - READY_STATUS=$(jq -r '.status.conditions[] | select(.type=="Ready") | .status' /tmp/rotation_response.json) - if [ "$READY_STATUS" != "Unknown" ]; then echo "FAIL: Ready status not reset to Unknown on rotation"; exit 1; fi + # Attempt to patch the expirationDate + if kubectl patch machineaccountkey test-key-auto -n $NAMESPACE --type=merge -p '{"spec":{"expirationDate":"2030-01-01T00:00:00Z"}}' 2> /tmp/immutability_err.json; then + echo "FAIL: spec update should have been rejected" + exit 1 + fi - echo "PASS: machine account key successfully rotated" + if grep -q "spec is immutable after creation" /tmp/immutability_err.json; then + echo "PASS: spec immutability validation triggered" + else + echo "FAIL: wrong error message" + cat /tmp/immutability_err.json + exit 1 + fi # Test 6: Validation - reject malformed public key - name: test-validation-malformed-key @@ -218,22 +213,5 @@ spec: echo "PASS: conditions are correctly initialized" - # Cleanup: delete created resources - - name: cleanup - description: Clean up test resources - try: - - delete: - ref: - apiVersion: iam.miloapis.com/v1alpha1 - kind: MachineAccount - name: test-machine-account - - delete: - ref: - apiVersion: identity.miloapis.com/v1alpha1 - kind: MachineAccountKey - name: test-key-auto - - delete: - ref: - apiVersion: identity.miloapis.com/v1alpha1 - kind: MachineAccountKey - name: test-key-provided + + From 9be63c02c0709f8ea7e4abc64fdf065655a9d7e9 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Sat, 28 Mar 2026 19:51:30 +0000 Subject: [PATCH 06/20] chore: add missing newlines at end of files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Automatically added newlines to 1 file(s) Co-Authored-By: github-actions[bot] --- config/samples/iam/v1alpha1/machineaccount.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/samples/iam/v1alpha1/machineaccount.yaml b/config/samples/iam/v1alpha1/machineaccount.yaml index 5b3c3ba8..9be0a33e 100644 --- a/config/samples/iam/v1alpha1/machineaccount.yaml +++ b/config/samples/iam/v1alpha1/machineaccount.yaml @@ -4,4 +4,4 @@ metadata: name: example-machine-account namespace: default spec: - state: Active \ No newline at end of file + state: Active From c022b902a40542905cd15cc5a40f63270c8089c5 Mon Sep 17 00:00:00 2001 From: Jose Szychowski Date: Tue, 31 Mar 2026 11:33:38 -0300 Subject: [PATCH 07/20] refactor: rename MachineAccountName to MachineAccountUserName in MachineAccountKeySpec to reflect email usage --- pkg/apis/identity/v1alpha1/machineaccountkey_types.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/apis/identity/v1alpha1/machineaccountkey_types.go b/pkg/apis/identity/v1alpha1/machineaccountkey_types.go index bce63926..46fd84d4 100644 --- a/pkg/apis/identity/v1alpha1/machineaccountkey_types.go +++ b/pkg/apis/identity/v1alpha1/machineaccountkey_types.go @@ -26,9 +26,9 @@ type MachineAccountKey struct { // MachineAccountKeySpec defines the desired state of MachineAccountKey type MachineAccountKeySpec struct { - // MachineAccountName is the name of the MachineAccount that owns this key. + // MachineAccountUserName is the email address of the MachineAccount that owns this key. // +kubebuilder:validation:Required - MachineAccountName string `json:"machineAccountName"` + MachineAccountUserName string `json:"machineAccountUserName"` // ExpirationDate is the date and time when the MachineAccountKey will expire. // If not specified, the MachineAccountKey will never expire. From 0c81ac9aacb1fbc9fc4bd4a3723c1f62daf245c0 Mon Sep 17 00:00:00 2001 From: Jose Szychowski Date: Tue, 31 Mar 2026 19:39:25 -0300 Subject: [PATCH 08/20] refactor: decouple machine account key storage from etcd by introducing a backend interface --- cmd/milo/apiserver/config.go | 38 +- cmd/milo/apiserver/server.go | 54 ++- config/apiserver/deployment.yaml | 13 + .../identity/machineaccountkeys/dynamic.go | 234 ++++++++++ .../identity/machineaccountkeys/rest.go | 258 +++++------ .../identity/machineaccountkeys/rest_test.go | 415 ------------------ .../identity/machineaccountkeys/strategy.go | 158 ------- .../storage/identity/storageprovider.go | 53 +-- .../identity/v1alpha1/zz_generated.openapi.go | 6 +- pkg/features/features.go | 11 + 10 files changed, 447 insertions(+), 793 deletions(-) create mode 100644 internal/apiserver/identity/machineaccountkeys/dynamic.go delete mode 100644 internal/apiserver/identity/machineaccountkeys/rest_test.go delete mode 100644 internal/apiserver/identity/machineaccountkeys/strategy.go diff --git a/cmd/milo/apiserver/config.go b/cmd/milo/apiserver/config.go index ebc29331..af823ccd 100644 --- a/cmd/milo/apiserver/config.go +++ b/cmd/milo/apiserver/config.go @@ -45,6 +45,7 @@ import ( "go.miloapis.com/milo/internal/apiserver/admission/initializer" eventsbackend "go.miloapis.com/milo/internal/apiserver/events" + machineaccountkeysbackend "go.miloapis.com/milo/internal/apiserver/identity/machineaccountkeys" sessionsbackend "go.miloapis.com/milo/internal/apiserver/identity/sessions" useridentitiesbackend "go.miloapis.com/milo/internal/apiserver/identity/useridentities" identitystorage "go.miloapis.com/milo/internal/apiserver/storage/identity" @@ -69,9 +70,10 @@ type Config struct { } type ExtraConfig struct { - SessionsProvider SessionsProviderConfig - UserIdentitiesProvider UserIdentitiesProviderConfig - EventsProvider EventsProviderConfig + SessionsProvider SessionsProviderConfig + UserIdentitiesProvider UserIdentitiesProviderConfig + MachineAccountKeysProvider MachineAccountKeysProviderConfig + EventsProvider EventsProviderConfig } // SessionsProviderConfig groups configuration for the sessions backend provider @@ -107,6 +109,17 @@ type EventsProviderConfig struct { ForwardExtras []string } +// MachineAccountKeysProviderConfig groups configuration for the machineaccountkeys backend provider +type MachineAccountKeysProviderConfig struct { + URL string + CAFile string + ClientCertFile string + ClientKeyFile string + TimeoutSeconds int + Retries int + ForwardExtras []string +} + type completedConfig struct { Options options.CompletedOptions @@ -199,6 +212,25 @@ func newIdentityStorageProvider(c *CompletedConfig) controlplaneapiserver.RESTSt provider.UserIdentities = backend } + if utilfeature.DefaultFeatureGate.Enabled(features.MachineAccountKeys) { + allow := make(map[string]struct{}, len(c.ExtraConfig.MachineAccountKeysProvider.ForwardExtras)) + for _, k := range c.ExtraConfig.MachineAccountKeysProvider.ForwardExtras { + allow[k] = struct{}{} + } + cfg := machineaccountkeysbackend.Config{ + BaseConfig: c.ControlPlane.Generic.LoopbackClientConfig, + ProviderURL: c.ExtraConfig.MachineAccountKeysProvider.URL, + CAFile: c.ExtraConfig.MachineAccountKeysProvider.CAFile, + ClientCertFile: c.ExtraConfig.MachineAccountKeysProvider.ClientCertFile, + ClientKeyFile: c.ExtraConfig.MachineAccountKeysProvider.ClientKeyFile, + Timeout: time.Duration(c.ExtraConfig.MachineAccountKeysProvider.TimeoutSeconds) * time.Second, + Retries: c.ExtraConfig.MachineAccountKeysProvider.Retries, + ExtrasAllow: allow, + } + backend, _ := machineaccountkeysbackend.NewDynamicProvider(cfg) + provider.MachineAccountKeys = backend + } + return provider } diff --git a/cmd/milo/apiserver/server.go b/cmd/milo/apiserver/server.go index 1e2e9648..542892d0 100644 --- a/cmd/milo/apiserver/server.go +++ b/cmd/milo/apiserver/server.go @@ -56,25 +56,29 @@ func init() { } var ( - SystemNamespace string - sessionsProviderURL string - sessionsProviderCAFile string - sessionsProviderClientCert string - sessionsProviderClientKey string - providerTimeoutSeconds int - providerRetries int - forwardExtras []string - userIdentitiesProviderURL string - userIdentitiesProviderCAFile string - userIdentitiesProviderClientCert string - userIdentitiesProviderClientKey string - eventsProviderURL string - eventsProviderCAFile string - eventsProviderClientCert string - eventsProviderClientKey string - eventsProviderTimeoutSeconds int - eventsProviderRetries int - eventsForwardExtras []string + SystemNamespace string + sessionsProviderURL string + sessionsProviderCAFile string + sessionsProviderClientCert string + sessionsProviderClientKey string + providerTimeoutSeconds int + providerRetries int + forwardExtras []string + userIdentitiesProviderURL string + userIdentitiesProviderCAFile string + userIdentitiesProviderClientCert string + userIdentitiesProviderClientKey string + machineAccountKeysProviderURL string + machineAccountKeysProviderCAFile string + machineAccountKeysProviderClientCert string + machineAccountKeysProviderClientKey string + eventsProviderURL string + eventsProviderCAFile string + eventsProviderClientCert string + eventsProviderClientKey string + eventsProviderTimeoutSeconds int + eventsProviderRetries int + eventsForwardExtras []string ) // NewCommand creates a *cobra.Command object with default parameters @@ -184,6 +188,10 @@ func NewCommand() *cobra.Command { fs.StringVar(&userIdentitiesProviderCAFile, "useridentities-provider-ca-file", "", "Path to CA file to validate useridentities provider TLS") fs.StringVar(&userIdentitiesProviderClientCert, "useridentities-provider-client-cert", "", "Client certificate for mTLS to useridentities provider") fs.StringVar(&userIdentitiesProviderClientKey, "useridentities-provider-client-key", "", "Client private key for mTLS to useridentities provider") + fs.StringVar(&machineAccountKeysProviderURL, "machineaccountkeys-provider-url", "", "Direct provider base URL for machineaccountkeys (e.g., https://zitadel-apiserver:8443)") + fs.StringVar(&machineAccountKeysProviderCAFile, "machineaccountkeys-provider-ca-file", "", "Path to CA file to validate machineaccountkeys provider TLS") + fs.StringVar(&machineAccountKeysProviderClientCert, "machineaccountkeys-provider-client-cert", "", "Client certificate for mTLS to machineaccountkeys provider") + fs.StringVar(&machineAccountKeysProviderClientKey, "machineaccountkeys-provider-client-key", "", "Client private key for mTLS to machineaccountkeys provider") fs.StringVar(&eventsProviderURL, "events-provider-url", "", "Activity API server URL for events storage (e.g., https://activity-apiserver.activity-system.svc:443)") fs.StringVar(&eventsProviderCAFile, "events-provider-ca-file", "", "Path to CA file to validate Activity provider TLS") fs.StringVar(&eventsProviderClientCert, "events-provider-client-cert", "", "Client certificate for mTLS to Activity provider") @@ -253,6 +261,14 @@ func Run(ctx context.Context, opts options.CompletedOptions) error { config.ExtraConfig.UserIdentitiesProvider.Retries = providerRetries config.ExtraConfig.UserIdentitiesProvider.ForwardExtras = forwardExtras + config.ExtraConfig.MachineAccountKeysProvider.URL = machineAccountKeysProviderURL + config.ExtraConfig.MachineAccountKeysProvider.CAFile = machineAccountKeysProviderCAFile + config.ExtraConfig.MachineAccountKeysProvider.ClientCertFile = machineAccountKeysProviderClientCert + config.ExtraConfig.MachineAccountKeysProvider.ClientKeyFile = machineAccountKeysProviderClientKey + config.ExtraConfig.MachineAccountKeysProvider.TimeoutSeconds = providerTimeoutSeconds + config.ExtraConfig.MachineAccountKeysProvider.Retries = providerRetries + config.ExtraConfig.MachineAccountKeysProvider.ForwardExtras = forwardExtras + config.ExtraConfig.EventsProvider.URL = eventsProviderURL config.ExtraConfig.EventsProvider.CAFile = eventsProviderCAFile config.ExtraConfig.EventsProvider.ClientCertFile = eventsProviderClientCert diff --git a/config/apiserver/deployment.yaml b/config/apiserver/deployment.yaml index b0a38258..e13dd69f 100644 --- a/config/apiserver/deployment.yaml +++ b/config/apiserver/deployment.yaml @@ -70,6 +70,11 @@ spec: - --useridentities-provider-ca-file=$(USERIDENTITIES_PROVIDER_CA_FILE) - --useridentities-provider-client-cert=$(USERIDENTITIES_PROVIDER_CLIENT_CERT_FILE) - --useridentities-provider-client-key=$(USERIDENTITIES_PROVIDER_CLIENT_KEY_FILE) + # MachineAccountKeys provider configuration + - --machineaccountkeys-provider-url=$(MACHINEACCOUNTKEYS_PROVIDER_URL) + - --machineaccountkeys-provider-ca-file=$(MACHINEACCOUNTKEYS_PROVIDER_CA_FILE) + - --machineaccountkeys-provider-client-cert=$(MACHINEACCOUNTKEYS_PROVIDER_CLIENT_CERT_FILE) + - --machineaccountkeys-provider-client-key=$(MACHINEACCOUNTKEYS_PROVIDER_CLIENT_KEY_FILE) # Events proxy provider configuration (requires EventsProxy feature gate) - --events-provider-url=$(EVENTS_PROVIDER_URL) - --events-provider-ca-file=$(EVENTS_PROVIDER_CA_FILE) @@ -156,6 +161,14 @@ spec: value: "" - name: USERIDENTITIES_PROVIDER_CLIENT_KEY_FILE value: "" + - name: MACHINEACCOUNTKEYS_PROVIDER_URL + value: "" + - name: MACHINEACCOUNTKEYS_PROVIDER_CA_FILE + value: "" + - name: MACHINEACCOUNTKEYS_PROVIDER_CLIENT_CERT_FILE + value: "" + - name: MACHINEACCOUNTKEYS_PROVIDER_CLIENT_KEY_FILE + value: "" # Events proxy provider configuration (requires --feature-gates=EventsProxy=true) - name: EVENTS_PROVIDER_URL value: "" diff --git a/internal/apiserver/identity/machineaccountkeys/dynamic.go b/internal/apiserver/identity/machineaccountkeys/dynamic.go new file mode 100644 index 00000000..7205c06c --- /dev/null +++ b/internal/apiserver/identity/machineaccountkeys/dynamic.go @@ -0,0 +1,234 @@ +package machineaccountkeys + +import ( + "context" + "fmt" + "net/http" + "net/url" + "time" + + identityv1alpha1 "go.miloapis.com/milo/pkg/apis/identity/v1alpha1" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + authuser "k8s.io/apiserver/pkg/authentication/user" + apirequest "k8s.io/apiserver/pkg/endpoints/request" + "k8s.io/client-go/dynamic" + "k8s.io/client-go/rest" + "k8s.io/client-go/transport" +) + +// Config controls how the provider talks to the remote machineaccountkeys API **always via a remote URL**. + +type Config struct { + BaseConfig *rest.Config + + ProviderURL string + + CAFile string + ClientCertFile string + ClientKeyFile string + + Timeout time.Duration + Retries int + ExtrasAllow map[string]struct{} +} + +// DynamicProvider implements Backend by proxying to a remote auth-provider +// that serves the machineaccountkeys API (e.g. auth-provider-zitadel). +type DynamicProvider struct { + base *rest.Config + gvr schema.GroupVersionResource + to time.Duration + retries int + allowExtras map[string]struct{} +} + +func NewDynamicProvider(cfg Config) (*DynamicProvider, error) { + if cfg.ProviderURL == "" { + return nil, fmt.Errorf("ProviderURL is required") + } + + // Build from scratch + base := &rest.Config{} + base.Host = cfg.ProviderURL + + var sni string + if u, err := url.Parse(cfg.ProviderURL); err == nil { + sni = u.Hostname() + } + + // Wire TLS from files + base.TLSClientConfig = rest.TLSClientConfig{ + CAFile: cfg.CAFile, + CertFile: cfg.ClientCertFile, + KeyFile: cfg.ClientKeyFile, + // We enforce verification; set Insecure=true only for dev + Insecure: false, + ServerName: sni, + } + + // Respect our explicit timeout + if cfg.Timeout > 0 { + base.Timeout = cfg.Timeout + } + + gvr := identityv1alpha1.SchemeGroupVersion.WithResource("machineaccountkeys") + + return &DynamicProvider{ + base: base, + gvr: gvr, + to: cfg.Timeout, + retries: max(0, cfg.Retries), + allowExtras: cfg.ExtrasAllow, + }, nil +} + +// dynForUser creates a per-call client-go dynamic.Interface that forwards identity via X-Remote-*. +func (b *DynamicProvider) dynForUser(ctx context.Context) (dynamic.Interface, error) { + u, ok := apirequest.UserFrom(ctx) + if !ok || u == nil { + return nil, fmt.Errorf("no user in context") + } + cfg := rest.CopyConfig(b.base) + if b.to > 0 { + cfg.Timeout = b.to + } + prev := cfg.WrapTransport + cfg.WrapTransport = func(rt http.RoundTripper) http.RoundTripper { + if prev != nil { + rt = prev(rt) + } + return transport.NewAuthProxyRoundTripper( + u.GetName(), + u.GetUID(), + u.GetGroups(), + b.filterExtras(u.GetExtra()), + rt, + ) + } + return dynamic.NewForConfig(cfg) +} + +func (b *DynamicProvider) filterExtras(src map[string][]string) map[string][]string { + if len(b.allowExtras) == 0 || len(src) == 0 { + return nil + } + out := make(map[string][]string, len(src)) + for k, v := range src { + if _, ok := b.allowExtras[k]; ok { + out[k] = v + } + } + return out +} + +// ---- Public API (implements Backend) ---- + +func (b *DynamicProvider) CreateMachineAccountKey(ctx context.Context, _ authuser.Info, key *identityv1alpha1.MachineAccountKey, opts *metav1.CreateOptions) (*identityv1alpha1.MachineAccountKey, error) { + dyn, err := b.dynForUser(ctx) + if err != nil { + return nil, err + } + if opts == nil { + opts = &metav1.CreateOptions{} + } + + // Convert to unstructured for the dynamic client + uobj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(key) + if err != nil { + return nil, fmt.Errorf("failed to convert MachineAccountKey to unstructured: %w", err) + } + + var lastErr error + var created *unstructured.Unstructured + for i := 0; i <= b.retries; i++ { + created, lastErr = dyn.Resource(b.gvr).Create(ctx, &unstructured.Unstructured{Object: uobj}, *opts) + if lastErr == nil { + break + } + } + if lastErr != nil { + return nil, lastErr + } + + out := new(identityv1alpha1.MachineAccountKey) + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(created.UnstructuredContent(), out); err != nil { + return nil, err + } + return out, nil +} + +func (b *DynamicProvider) ListMachineAccountKeys(ctx context.Context, _ authuser.Info, opts *metav1.ListOptions) (*identityv1alpha1.MachineAccountKeyList, error) { + if opts == nil { + opts = &metav1.ListOptions{} + } + dyn, err := b.dynForUser(ctx) + if err != nil { + return nil, err + } + var lastErr error + var ul *unstructured.UnstructuredList + for i := 0; i <= b.retries; i++ { + ul, lastErr = dyn.Resource(b.gvr).List(ctx, *opts) + if lastErr == nil { + break + } + } + if lastErr != nil { + return nil, lastErr + } + out := new(identityv1alpha1.MachineAccountKeyList) + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(ul.UnstructuredContent(), out); err != nil { + return nil, err + } + return out, nil +} + +func (b *DynamicProvider) GetMachineAccountKey(ctx context.Context, _ authuser.Info, name string) (*identityv1alpha1.MachineAccountKey, error) { + dyn, err := b.dynForUser(ctx) + if err != nil { + return nil, err + } + var lastErr error + var uobj *unstructured.Unstructured + for i := 0; i <= b.retries; i++ { + uobj, lastErr = dyn.Resource(b.gvr).Get(ctx, name, metav1.GetOptions{}) + if lastErr == nil { + break + } + } + if lastErr != nil { + return nil, lastErr + } + out := new(identityv1alpha1.MachineAccountKey) + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(uobj.UnstructuredContent(), out); err != nil { + return nil, err + } + return out, nil +} + +func (b *DynamicProvider) DeleteMachineAccountKey(ctx context.Context, _ authuser.Info, name string) error { + dyn, err := b.dynForUser(ctx) + if err != nil { + return err + } + var lastErr error + for i := 0; i <= b.retries; i++ { + lastErr = dyn.Resource(b.gvr).Delete(ctx, name, metav1.DeleteOptions{}) + if lastErr == nil { + return nil + } + } + return lastErr +} + +// small util +func max(a, b int) int { + if a > b { + return a + } + return b +} diff --git a/internal/apiserver/identity/machineaccountkeys/rest.go b/internal/apiserver/identity/machineaccountkeys/rest.go index 7a871fd8..4c75290e 100644 --- a/internal/apiserver/identity/machineaccountkeys/rest.go +++ b/internal/apiserver/identity/machineaccountkeys/rest.go @@ -2,203 +2,167 @@ package machineaccountkeys import ( "context" - "crypto/rand" - "crypto/rsa" - "crypto/x509" - "encoding/pem" - "fmt" "time" apierrors "k8s.io/apimachinery/pkg/api/errors" + metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - generic "k8s.io/apiserver/pkg/registry/generic" - genericregistry "k8s.io/apiserver/pkg/registry/generic/registry" + authuser "k8s.io/apiserver/pkg/authentication/user" + apirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/rest" - k8smetrics "k8s.io/component-base/metrics" - k8slegacy "k8s.io/component-base/metrics/legacyregistry" + "k8s.io/klog/v2" identityv1alpha1 "go.miloapis.com/milo/pkg/apis/identity/v1alpha1" ) -// generateRSAKeyPairFunc is a package-level variable to allow test overriding. -var generateRSAKeyPairFunc = generateRSAKeyPair - -// metrics for key generation observability (NFR3) -var ( - keyGenerationDuration = k8smetrics.NewHistogramVec( - &k8smetrics.HistogramOpts{ - Name: "milo_machineaccountkey_generation_duration_seconds", - Help: "Duration of RSA key generation for MachineAccountKey creation", - Buckets: []float64{0.01, 0.025, 0.05, 0.1, 0.25, 0.5, 1.0}, - StabilityLevel: k8smetrics.ALPHA, - }, - []string{"result"}, // "success" | "failure" - ) -) - -func init() { - k8slegacy.MustRegister(keyGenerationDuration) +// Backend is the interface that the REST handler delegates all operations to. +// Implementations proxy requests to the auth-provider (e.g. Zitadel) service. +type Backend interface { + CreateMachineAccountKey(ctx context.Context, u authuser.Info, key *identityv1alpha1.MachineAccountKey, opts *metav1.CreateOptions) (*identityv1alpha1.MachineAccountKey, error) + ListMachineAccountKeys(ctx context.Context, u authuser.Info, opts *metav1.ListOptions) (*identityv1alpha1.MachineAccountKeyList, error) + GetMachineAccountKey(ctx context.Context, u authuser.Info, name string) (*identityv1alpha1.MachineAccountKey, error) + DeleteMachineAccountKey(ctx context.Context, u authuser.Info, name string) error } -// REST wraps a genericregistry.Store and intercepts Create to inject RSA key generation. -// It embeds the store to inherit all standard REST operations (Get, List, Watch, Delete, -// ConvertToTable, etc.) and only overrides Create. type REST struct { - *genericregistry.Store + backend Backend } var _ rest.Scoper = &REST{} -var _ rest.Creater = &REST{} -var _ rest.Updater = &REST{} -var _ rest.Getter = &REST{} +var _ rest.Creater = &REST{} //nolint:misspell var _ rest.Lister = &REST{} +var _ rest.Getter = &REST{} var _ rest.GracefulDeleter = &REST{} -var _ rest.Watcher = &REST{} var _ rest.Storage = &REST{} var _ rest.SingularNameProvider = &REST{} -// NewREST constructs a REST storage handler backed by etcd via the given RESTOptionsGetter. -// It returns an error if the store cannot be completed (e.g. RESTOptionsGetter is misconfigured). -func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, error) { - gr := identityv1alpha1.SchemeGroupVersion.WithResource("machineaccountkeys").GroupResource() - - store := &genericregistry.Store{ - NewFunc: func() runtime.Object { return &identityv1alpha1.MachineAccountKey{} }, - NewListFunc: func() runtime.Object { return &identityv1alpha1.MachineAccountKeyList{} }, - DefaultQualifiedResource: gr, - SingularQualifiedResource: identityv1alpha1.SchemeGroupVersion.WithResource("machineaccountkey").GroupResource(), - CreateStrategy: Strategy, - UpdateStrategy: Strategy, - DeleteStrategy: Strategy, - TableConvertor: rest.NewDefaultTableConvertor(gr), - } - - options := &generic.StoreOptions{ - RESTOptions: optsGetter, - } - - if err := store.CompleteWithOptions(options); err != nil { - return nil, fmt.Errorf("failed to complete machineaccountkeys store: %w", err) - } - - return &REST{Store: store}, nil -} +func NewREST(b Backend) *REST { return &REST{backend: b} } func (r *REST) GetSingularName() string { return "machineaccountkey" } +func (r *REST) NamespaceScoped() bool { return false } +func (r *REST) New() runtime.Object { return &identityv1alpha1.MachineAccountKey{} } +func (r *REST) NewList() runtime.Object { return &identityv1alpha1.MachineAccountKeyList{} } -// Create intercepts the standard create path to optionally generate an RSA key pair -// when spec.publicKey is omitted. The private key is returned in status.privateKey -// of the HTTP response object only — it is never passed to etcd storage. func (r *REST) Create( ctx context.Context, obj runtime.Object, - createValidation rest.ValidateObjectFunc, - options *metav1.CreateOptions, + _ rest.ValidateObjectFunc, + opts *metav1.CreateOptions, ) (runtime.Object, error) { + logger := klog.FromContext(ctx) + u, _ := apirequest.UserFrom(ctx) key, ok := obj.(*identityv1alpha1.MachineAccountKey) if !ok { - return nil, apierrors.NewBadRequest(fmt.Sprintf( - "not a MachineAccountKey: %T", obj, - )) + return nil, apierrors.NewBadRequest("not a MachineAccountKey") } - - // FR6: validate expiration date is in the future if provided - if key.Spec.ExpirationDate != nil && !key.Spec.ExpirationDate.Time.IsZero() { - if !key.Spec.ExpirationDate.Time.After(time.Now()) { - return nil, apierrors.NewBadRequest("spec.expirationDate must be in the future") - } + logger.V(4).Info("Creating machine account key", "name", key.Name, "machineAccount", key.Spec.MachineAccountUserName) + res, err := r.backend.CreateMachineAccountKey(ctx, u, key, opts) + if err != nil { + logger.Error(err, "Create machine account key failed", "name", key.Name) + return nil, err } + logger.V(4).Info("Created machine account key", "name", res.Name, "authProviderKeyID", res.Status.AuthProviderKeyID) + return res, nil +} - // FR7: validate public key format if provided - if key.Spec.PublicKey != "" { - if err := validateRSAPublicKey(key.Spec.PublicKey); err != nil { - return nil, err - } +func (r *REST) List(ctx context.Context, opts *metainternalversion.ListOptions) (runtime.Object, error) { + logger := klog.FromContext(ctx) + u, _ := apirequest.UserFrom(ctx) + username, uid, groups := "", "", []string(nil) + if u != nil { + username = u.GetName() + uid = u.GetUID() + groups = u.GetGroups() } - - // FR1: auto-generate RSA 2048-bit key pair when no public key is provided - var privateKeyPEM string - if key.Spec.PublicKey == "" { - start := time.Now() - pubPEM, privPEM, err := generateRSAKeyPairFunc() - elapsed := time.Since(start) - - if err != nil { - keyGenerationDuration.WithLabelValues("failure").Observe(elapsed.Seconds()) - return nil, apierrors.NewInternalError(fmt.Errorf("failed to generate RSA key pair: %w", err)) - } - keyGenerationDuration.WithLabelValues("success").Observe(elapsed.Seconds()) - - key.Spec.PublicKey = pubPEM - privateKeyPEM = privPEM + logger.V(4).Info("Listing machine account keys", "username", username, "uid", uid, "groups", groups) + lo := metav1.ListOptions{} + if opts != nil && opts.FieldSelector != nil && !opts.FieldSelector.Empty() { + lo.FieldSelector = opts.FieldSelector.String() } - - // Persist to etcd. The strategy's PrepareForCreate ensures Status.PrivateKey - // is cleared before the object reaches storage (defense in depth). - result, err := r.Store.Create(ctx, key, createValidation, options) + res, err := r.backend.ListMachineAccountKeys(ctx, u, &lo) if err != nil { + logger.Error(err, "List machine account keys failed") return nil, err } - - // FR1/FR3: Set the private key on the in-memory response object AFTER the - // etcd write returns. The object stored in etcd never carries the private key. - if privateKeyPEM != "" { - createdKey, ok := result.(*identityv1alpha1.MachineAccountKey) - if ok { - createdKey.Status.PrivateKey = privateKeyPEM - } - } - - return result, nil + logger.V(4).Info("Listed machine account keys", "count", len(res.Items)) + return res, nil } -// Update is omitted here as it's not needed. The embedded genericregistry.Store.Update will be used. -// The strategy's ValidateUpdate will still be called to enforce Spec immutability. - -// generateRSAKeyPair generates a 2048-bit RSA key pair and returns -// PEM-encoded public (PKIX) and private (PKCS1) key material. -func generateRSAKeyPair() (publicKeyPEM string, privateKeyPEM string, err error) { - privateKey, err := rsa.GenerateKey(rand.Reader, 2048) - if err != nil { - return "", "", fmt.Errorf("rsa.GenerateKey: %w", err) +func (r *REST) Get(ctx context.Context, name string, _ *metav1.GetOptions) (runtime.Object, error) { + logger := klog.FromContext(ctx) + u, _ := apirequest.UserFrom(ctx) + username, uid := "", "" + if u != nil { + username = u.GetName() + uid = u.GetUID() } - - pubDER, err := x509.MarshalPKIXPublicKey(&privateKey.PublicKey) + logger.V(4).Info("Getting machine account key", "name", name, "username", username, "uid", uid) + res, err := r.backend.GetMachineAccountKey(ctx, u, name) if err != nil { - return "", "", fmt.Errorf("x509.MarshalPKIXPublicKey: %w", err) + logger.Error(err, "Get machine account key failed", "name", name) + return nil, err } + logger.V(4).Info("Got machine account key", "name", name, "authProviderKeyID", res.Status.AuthProviderKeyID) + return res, nil +} - pubPEMBlock := pem.EncodeToMemory(&pem.Block{ - Type: "PUBLIC KEY", - Bytes: pubDER, - }) - - privDER := x509.MarshalPKCS1PrivateKey(privateKey) - privPEMBlock := pem.EncodeToMemory(&pem.Block{ - Type: "RSA PRIVATE KEY", - Bytes: privDER, - }) - - return string(pubPEMBlock), string(privPEMBlock), nil +func (r *REST) Delete(ctx context.Context, name string, _ rest.ValidateObjectFunc, _ *metav1.DeleteOptions) (runtime.Object, bool, error) { + logger := klog.FromContext(ctx) + u, _ := apirequest.UserFrom(ctx) + username, uid := "", "" + if u != nil { + username = u.GetName() + uid = u.GetUID() + } + logger.V(4).Info("Deleting machine account key", "name", name, "username", username, "uid", uid) + if err := r.backend.DeleteMachineAccountKey(ctx, u, name); err != nil { + logger.Error(err, "Delete machine account key failed", "name", name) + return nil, false, err + } + logger.V(4).Info("Deleted machine account key", "name", name) + return &metav1.Status{Status: metav1.StatusSuccess}, true, nil } -// validateRSAPublicKey returns an API error if the PEM string is not a valid -// PEM-encoded RSA public key. It returns nil when the key is acceptable. -func validateRSAPublicKey(pubKeyPEM string) error { - block, _ := pem.Decode([]byte(pubKeyPEM)) - if block == nil { - return apierrors.NewBadRequest("spec.publicKey must be PEM-encoded") +func (r *REST) Destroy() {} + +// ConvertToTable satisfies rest.TableConvertor with a kubectl-friendly table output. +func (r *REST) ConvertToTable(ctx context.Context, object runtime.Object, tableOptions runtime.Object) (*metav1.Table, error) { + table := &metav1.Table{ + ColumnDefinitions: []metav1.TableColumnDefinition{ + {Name: "Name", Type: "string"}, + {Name: "Machine Account", Type: "string"}, + {Name: "Key ID", Type: "string"}, + {Name: "Age", Type: "date"}, + {Name: "Expires", Type: "string"}, + }, } - pub, err := x509.ParsePKIXPublicKey(block.Bytes) - if err != nil { - return apierrors.NewBadRequest(fmt.Sprintf("invalid public key: %v", err)) + appendRow := func(mak *identityv1alpha1.MachineAccountKey) { + age := metav1.Now().Rfc3339Copy() + if !mak.CreationTimestamp.IsZero() { + age = mak.CreationTimestamp + } + expiresStr := "" + if mak.Spec.ExpirationDate != nil { + expiresStr = mak.Spec.ExpirationDate.Time.Format(time.RFC3339) + } + table.Rows = append(table.Rows, metav1.TableRow{ + Cells: []interface{}{mak.Name, mak.Spec.MachineAccountUserName, mak.Status.AuthProviderKeyID, age.Time.Format(time.RFC3339), expiresStr}, + Object: runtime.RawExtension{Object: mak}, + }) } - if _, ok := pub.(*rsa.PublicKey); !ok { - return apierrors.NewBadRequest("only RSA public keys are supported in v1alpha1") + switch obj := object.(type) { + case *identityv1alpha1.MachineAccountKeyList: + for i := range obj.Items { + appendRow(&obj.Items[i]) + } + case *identityv1alpha1.MachineAccountKey: + appendRow(obj) + default: + return nil, nil } - return nil + return table, nil } diff --git a/internal/apiserver/identity/machineaccountkeys/rest_test.go b/internal/apiserver/identity/machineaccountkeys/rest_test.go deleted file mode 100644 index b807feb7..00000000 --- a/internal/apiserver/identity/machineaccountkeys/rest_test.go +++ /dev/null @@ -1,415 +0,0 @@ -package machineaccountkeys - -import ( - "context" - "crypto/rand" - "crypto/rsa" - "crypto/x509" - "encoding/pem" - "errors" - "testing" - "time" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - apierrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - apirequest "k8s.io/apiserver/pkg/endpoints/request" - "k8s.io/apiserver/pkg/registry/rest" - - identityv1alpha1 "go.miloapis.com/milo/pkg/apis/identity/v1alpha1" -) - -// mockStore is a minimal fake underlying store for Create. It captures what -// object was passed to Create so tests can verify the private key was not persisted. -type mockStore struct { - capturedObj runtime.Object - returnObj runtime.Object - returnErr error -} - -func (m *mockStore) Create( - _ context.Context, - obj runtime.Object, - _ rest.ValidateObjectFunc, - _ *metav1.CreateOptions, -) (runtime.Object, error) { - // Deep-copy to capture the state at the time of the store call. - m.capturedObj = obj.DeepCopyObject() - if m.returnErr != nil { - return nil, m.returnErr - } - if m.returnObj != nil { - return m.returnObj, nil - } - return obj, nil -} - -// restWithMockStore builds a REST instance whose embedded Store.Create is replaced -// by a lightweight test double. We bypass NewREST to avoid needing an etcd backend -// in unit tests; only the Create interception logic is under test. -type testREST struct { - store *mockStore -} - -func (r *testREST) Create( - ctx context.Context, - obj runtime.Object, - createValidation rest.ValidateObjectFunc, - options *metav1.CreateOptions, -) (runtime.Object, error) { - key, ok := obj.(*identityv1alpha1.MachineAccountKey) - if !ok { - return nil, apierrors.NewBadRequest("not a MachineAccountKey") - } - - if key.Spec.ExpirationDate != nil && !key.Spec.ExpirationDate.Time.IsZero() { - if !key.Spec.ExpirationDate.Time.After(time.Now()) { - return nil, apierrors.NewBadRequest("spec.expirationDate must be in the future") - } - } - - if key.Spec.PublicKey != "" { - if err := validateRSAPublicKey(key.Spec.PublicKey); err != nil { - return nil, err - } - } - - var privateKeyPEM string - if key.Spec.PublicKey == "" { - pubPEM, privPEM, err := generateRSAKeyPairFunc() - if err != nil { - return nil, apierrors.NewInternalError(err) - } - key.Spec.PublicKey = pubPEM - privateKeyPEM = privPEM - } - - // Simulate strategy PrepareForCreate: clear private key before storage write. - key.Status.PrivateKey = "" - - result, err := r.store.Create(ctx, key, createValidation, options) - if err != nil { - return nil, err - } - - if privateKeyPEM != "" { - createdKey, ok := result.(*identityv1alpha1.MachineAccountKey) - if ok { - createdKey.Status.PrivateKey = privateKeyPEM - } - } - - return result, nil -} - -func newTestREST(store *mockStore) *testREST { - return &testREST{store: store} -} - -func makeKey(name, machineAccountName, publicKey string) *identityv1alpha1.MachineAccountKey { - return &identityv1alpha1.MachineAccountKey{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: "default", - }, - Spec: identityv1alpha1.MachineAccountKeySpec{ - MachineAccountName: machineAccountName, - PublicKey: publicKey, - }, - } -} - -func generateTestRSAPublicKeyPEM(t *testing.T) string { - t.Helper() - priv, err := rsa.GenerateKey(rand.Reader, 2048) - require.NoError(t, err) - pubDER, err := x509.MarshalPKIXPublicKey(&priv.PublicKey) - require.NoError(t, err) - return string(pem.EncodeToMemory(&pem.Block{Type: "PUBLIC KEY", Bytes: pubDER})) -} - -func TestCreate_AutoGeneration(t *testing.T) { - store := &mockStore{} - r := newTestREST(store) - - key := makeKey("test-key", "my-machine-account", "") - - ctx := apirequest.WithNamespace(context.Background(), "default") - result, err := r.Create(ctx, key, nil, &metav1.CreateOptions{}) - - require.NoError(t, err) - require.NotNil(t, result) - - createdKey, ok := result.(*identityv1alpha1.MachineAccountKey) - require.True(t, ok) - - // The response must contain a private key. - assert.NotEmpty(t, createdKey.Status.PrivateKey, "private key should appear in the creation response") - assert.Contains(t, createdKey.Status.PrivateKey, "-----BEGIN RSA PRIVATE KEY-----", - "private key should be PEM-encoded PKCS1") - - // The spec must contain a public key. - assert.NotEmpty(t, createdKey.Spec.PublicKey, "public key should be populated in spec") - assert.Contains(t, createdKey.Spec.PublicKey, "-----BEGIN PUBLIC KEY-----") -} - -func TestCreate_AutoGeneration_PrivateKeyAbsentFromStoredObject(t *testing.T) { - store := &mockStore{} - r := newTestREST(store) - - key := makeKey("test-key", "my-machine-account", "") - - ctx := apirequest.WithNamespace(context.Background(), "default") - _, err := r.Create(ctx, key, nil, &metav1.CreateOptions{}) - require.NoError(t, err) - - // The object captured by the store (i.e., what would go to etcd) must have - // no private key — this is the critical security invariant. - stored, ok := store.capturedObj.(*identityv1alpha1.MachineAccountKey) - require.True(t, ok) - assert.Empty(t, stored.Status.PrivateKey, - "private key must NOT be present in the object passed to storage (etcd)") -} - -func TestCreate_ProvidedPublicKey_NoPrivateKeyInResponse(t *testing.T) { - pubKeyPEM := generateTestRSAPublicKeyPEM(t) - - store := &mockStore{} - r := newTestREST(store) - - key := makeKey("test-key", "my-machine-account", pubKeyPEM) - - ctx := apirequest.WithNamespace(context.Background(), "default") - result, err := r.Create(ctx, key, nil, &metav1.CreateOptions{}) - - require.NoError(t, err) - require.NotNil(t, result) - - createdKey, ok := result.(*identityv1alpha1.MachineAccountKey) - require.True(t, ok) - - // No private key should appear when the caller supplies their own public key. - assert.Empty(t, createdKey.Status.PrivateKey, - "status.privateKey should be absent when publicKey is provided") - - // The provided public key should be preserved unchanged. - assert.Equal(t, pubKeyPEM, createdKey.Spec.PublicKey, - "provided public key should be passed through unchanged") -} - -func TestCreate_GenerationFailure_Returns500(t *testing.T) { - // Override the key generation function to simulate a failure. - original := generateRSAKeyPairFunc - defer func() { generateRSAKeyPairFunc = original }() - generateRSAKeyPairFunc = func() (string, string, error) { - return "", "", errors.New("entropy source unavailable") - } - - store := &mockStore{} - r := newTestREST(store) - - key := makeKey("test-key", "my-machine-account", "") - - ctx := apirequest.WithNamespace(context.Background(), "default") - _, err := r.Create(ctx, key, nil, &metav1.CreateOptions{}) - - require.Error(t, err) - statusErr := &apierrors.StatusError{} - require.True(t, errors.As(err, &statusErr)) - assert.Equal(t, int32(500), statusErr.ErrStatus.Code, - "key generation failure should return 500 Internal Server Error") -} - -func TestCreate_StorageFailure_Propagated(t *testing.T) { - expectedErr := errors.New("etcd connection refused") - store := &mockStore{returnErr: expectedErr} - r := newTestREST(store) - - key := makeKey("test-key", "my-machine-account", "") - - ctx := apirequest.WithNamespace(context.Background(), "default") - _, err := r.Create(ctx, key, nil, &metav1.CreateOptions{}) - - require.Error(t, err) - assert.Equal(t, expectedErr, err, "storage errors should propagate unchanged") -} - -func TestCreate_MalformedPublicKey_Returns400(t *testing.T) { - store := &mockStore{} - r := newTestREST(store) - - key := makeKey("test-key", "my-machine-account", "not-a-valid-pem") - - ctx := apirequest.WithNamespace(context.Background(), "default") - _, err := r.Create(ctx, key, nil, &metav1.CreateOptions{}) - - require.Error(t, err) - statusErr := &apierrors.StatusError{} - require.True(t, errors.As(err, &statusErr)) - assert.Equal(t, int32(400), statusErr.ErrStatus.Code) -} - -func TestCreate_CorruptPublicKeyDER_Returns400(t *testing.T) { - // Build a PEM block that decodes correctly but contains truncated DER bytes, - // causing x509.ParsePKIXPublicKey to fail. This exercises the "invalid public key" - // error path in validateRSAPublicKey. - priv, err := rsa.GenerateKey(rand.Reader, 2048) - require.NoError(t, err) - pubDER, err := x509.MarshalPKIXPublicKey(&priv.PublicKey) - require.NoError(t, err) - - corruptPEM := string(pem.EncodeToMemory(&pem.Block{ - Type: "PUBLIC KEY", - Bytes: pubDER[:len(pubDER)/2], // truncate to corrupt the DER - })) - - store := &mockStore{} - r := newTestREST(store) - - key := makeKey("test-key", "my-machine-account", corruptPEM) - - ctx := apirequest.WithNamespace(context.Background(), "default") - _, createErr := r.Create(ctx, key, nil, &metav1.CreateOptions{}) - - require.Error(t, createErr) - statusErr := &apierrors.StatusError{} - require.True(t, errors.As(createErr, &statusErr)) - assert.Equal(t, int32(400), statusErr.ErrStatus.Code) -} - -func TestCreate_ExpirationDateInPast_Returns400(t *testing.T) { - store := &mockStore{} - r := newTestREST(store) - - pastTime := metav1.NewTime(time.Now().Add(-24 * time.Hour)) - key := makeKey("test-key", "my-machine-account", "") - key.Spec.ExpirationDate = &pastTime - - ctx := apirequest.WithNamespace(context.Background(), "default") - _, err := r.Create(ctx, key, nil, &metav1.CreateOptions{}) - - require.Error(t, err) - statusErr := &apierrors.StatusError{} - require.True(t, errors.As(err, &statusErr)) - assert.Equal(t, int32(400), statusErr.ErrStatus.Code) - assert.Contains(t, statusErr.ErrStatus.Message, "expirationDate") -} - -func TestCreate_ExpirationDateInFuture_Succeeds(t *testing.T) { - store := &mockStore{} - r := newTestREST(store) - - futureTime := metav1.NewTime(time.Now().Add(24 * time.Hour)) - key := makeKey("test-key", "my-machine-account", "") - key.Spec.ExpirationDate = &futureTime - - ctx := apirequest.WithNamespace(context.Background(), "default") - result, err := r.Create(ctx, key, nil, &metav1.CreateOptions{}) - - require.NoError(t, err) - require.NotNil(t, result) -} - -func TestValidateRSAPublicKey_ValidKey_ReturnsNil(t *testing.T) { - pubKeyPEM := generateTestRSAPublicKeyPEM(t) - err := validateRSAPublicKey(pubKeyPEM) - assert.NoError(t, err) -} - -func TestValidateRSAPublicKey_NotPEM_Returns400(t *testing.T) { - err := validateRSAPublicKey("this is not PEM") - require.Error(t, err) - statusErr := &apierrors.StatusError{} - require.True(t, errors.As(err, &statusErr)) - assert.Equal(t, int32(400), statusErr.ErrStatus.Code) -} - -func TestValidateRSAPublicKey_CorruptDER_Returns400(t *testing.T) { - corruptPEM := string(pem.EncodeToMemory(&pem.Block{ - Type: "PUBLIC KEY", - Bytes: []byte("not valid DER"), - })) - err := validateRSAPublicKey(corruptPEM) - require.Error(t, err) - statusErr := &apierrors.StatusError{} - require.True(t, errors.As(err, &statusErr)) - assert.Equal(t, int32(400), statusErr.ErrStatus.Code) -} - -func TestGenerateRSAKeyPair_ProducesValidKeys(t *testing.T) { - pubPEM, privPEM, err := generateRSAKeyPair() - require.NoError(t, err) - - // Validate public key. - pubBlock, _ := pem.Decode([]byte(pubPEM)) - require.NotNil(t, pubBlock, "public key should be valid PEM") - assert.Equal(t, "PUBLIC KEY", pubBlock.Type) - pub, err := x509.ParsePKIXPublicKey(pubBlock.Bytes) - require.NoError(t, err) - _, ok := pub.(*rsa.PublicKey) - assert.True(t, ok, "public key should be RSA") - - // Validate private key. - privBlock, _ := pem.Decode([]byte(privPEM)) - require.NotNil(t, privBlock, "private key should be valid PEM") - assert.Equal(t, "RSA PRIVATE KEY", privBlock.Type) - priv, err := x509.ParsePKCS1PrivateKey(privBlock.Bytes) - require.NoError(t, err) - assert.Equal(t, 2048, priv.Size()*8, "key should be 2048-bit") -} - -func TestValidateUpdate_BlocksSpecChange(t *testing.T) { - oldKey := makeKey("test-key", "original-account", "") - newKey := makeKey("test-key", "changed-account", "") - - errs := Strategy.ValidateUpdate(context.Background(), newKey, oldKey) - - require.Len(t, errs, 1) - assert.Equal(t, "spec", errs[0].Field) - assert.Contains(t, errs[0].Detail, "immutable") -} - -func TestValidateUpdate_BlocksPublicKeyRotation(t *testing.T) { - oldPubKey := generateTestRSAPublicKeyPEM(t) - newPubKey := generateTestRSAPublicKeyPEM(t) - - oldKey := makeKey("test-key", "my-account", oldPubKey) - newKey := makeKey("test-key", "my-account", newPubKey) - - errs := Strategy.ValidateUpdate(context.Background(), newKey, oldKey) - - require.Len(t, errs, 1) - assert.Equal(t, "spec", errs[0].Field) - assert.Contains(t, errs[0].Detail, "immutable") -} - -func TestValidateUpdate_BlocksPublicKeyToEmptyString(t *testing.T) { - oldPubKey := generateTestRSAPublicKeyPEM(t) - - oldKey := makeKey("test-key", "my-account", oldPubKey) - newKey := makeKey("test-key", "my-account", "") - - errs := Strategy.ValidateUpdate(context.Background(), newKey, oldKey) - - require.Len(t, errs, 1) - assert.Equal(t, "spec", errs[0].Field) - assert.Contains(t, errs[0].Detail, "immutable") -} - -func TestValidateUpdate_NoChanges_Succeeds(t *testing.T) { - pubKey := generateTestRSAPublicKeyPEM(t) - expTime := metav1.NewTime(time.Date(2027, 1, 1, 0, 0, 0, 0, time.UTC)) - - oldKey := makeKey("test-key", "my-account", pubKey) - oldKey.Spec.ExpirationDate = &expTime - - newKey := makeKey("test-key", "my-account", pubKey) - newKey.Spec.ExpirationDate = &expTime - - errs := Strategy.ValidateUpdate(context.Background(), newKey, oldKey) - - require.Len(t, errs, 0, "no-op update should succeed") -} diff --git a/internal/apiserver/identity/machineaccountkeys/strategy.go b/internal/apiserver/identity/machineaccountkeys/strategy.go deleted file mode 100644 index 94174659..00000000 --- a/internal/apiserver/identity/machineaccountkeys/strategy.go +++ /dev/null @@ -1,158 +0,0 @@ -package machineaccountkeys - -import ( - "context" - "time" - - apiequality "k8s.io/apimachinery/pkg/api/equality" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/util/validation/field" - "k8s.io/apiserver/pkg/registry/rest" - "k8s.io/apiserver/pkg/storage/names" - "k8s.io/kubernetes/pkg/api/legacyscheme" - - identityv1alpha1 "go.miloapis.com/milo/pkg/apis/identity/v1alpha1" -) - -// machineAccountKeyStrategy implements rest.RESTCreateStrategy for MachineAccountKey. -type machineAccountKeyStrategy struct { - runtime.ObjectTyper - names.NameGenerator -} - -// Strategy is the singleton strategy instance used by the REST handler's store. -// It uses legacyscheme.Scheme as the ObjectTyper so the store can identify objects -// by their registered GVK. The identity scheme must be installed into legacyscheme.Scheme -// (via identityapi.Install in config.go) before this is used. -var Strategy = machineAccountKeyStrategy{legacyscheme.Scheme, names.SimpleNameGenerator} - -var _ rest.RESTCreateStrategy = machineAccountKeyStrategy{} -var _ rest.RESTUpdateStrategy = machineAccountKeyStrategy{} -var _ rest.RESTDeleteStrategy = machineAccountKeyStrategy{} - -func (machineAccountKeyStrategy) NamespaceScoped() bool { return true } - -// PrepareForCreate clears the PrivateKey field before the object is written to -// etcd. This is the primary defense-in-depth mechanism ensuring the private key -// is never persisted, even if the REST handler incorrectly sets it before calling -// the underlying store. -func (machineAccountKeyStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) { - key, ok := obj.(*identityv1alpha1.MachineAccountKey) - if !ok { - return - } - - // Never persist the private key to etcd. - key.Status.PrivateKey = "" - - // FR8: initialize Ready=Unknown if no conditions are set - if len(key.Status.Conditions) == 0 { - key.Status.Conditions = []metav1.Condition{ - { - Type: "Ready", - Status: metav1.ConditionUnknown, - Reason: "Unknown", - Message: "Waiting for control plane to reconcile", - LastTransitionTime: metav1.NewTime(time.Unix(0, 0).UTC()), - }, - } - } -} - -// PrepareForUpdate clears the PrivateKey field before the object is written to -// etcd (same as PrepareForCreate) to prevent accidental persistence. -func (machineAccountKeyStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) { - key, ok := obj.(*identityv1alpha1.MachineAccountKey) - if !ok { - return - } - - // Never persist the private key to etcd (defense in depth). - key.Status.PrivateKey = "" -} - -// Validate enforces field-level constraints on MachineAccountKey before persistence. -func (machineAccountKeyStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { - key, ok := obj.(*identityv1alpha1.MachineAccountKey) - if !ok { - return field.ErrorList{field.InternalError(field.NewPath(""), nil)} - } - - var errs field.ErrorList - specPath := field.NewPath("spec") - - // FR5: machineAccountName must be non-empty - if key.Spec.MachineAccountName == "" { - errs = append(errs, field.Required(specPath.Child("machineAccountName"), "machineAccountName is required")) - } - - // FR6: expiration date must be in the future if provided - if key.Spec.ExpirationDate != nil && !key.Spec.ExpirationDate.Time.IsZero() { - if !key.Spec.ExpirationDate.Time.After(time.Now()) { - errs = append(errs, field.Invalid( - specPath.Child("expirationDate"), - key.Spec.ExpirationDate, - "expirationDate must be in the future", - )) - } - } - - // FR7: public key must be a valid PEM-encoded RSA public key if provided - if key.Spec.PublicKey != "" { - if err := validateRSAPublicKey(key.Spec.PublicKey); err != nil { - errs = append(errs, field.Invalid( - specPath.Child("publicKey"), - "", - err.Error(), - )) - } - } - - return errs -} - -// ValidateUpdate enforces immutability constraints and validates updates to MachineAccountKey. -// It blocks any updates to the Spec after creation. -func (machineAccountKeyStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { - key, ok := obj.(*identityv1alpha1.MachineAccountKey) - if !ok { - return field.ErrorList{field.InternalError(field.NewPath(""), nil)} - } - - oldKey, ok := old.(*identityv1alpha1.MachineAccountKey) - if !ok { - return field.ErrorList{field.InternalError(field.NewPath(""), nil)} - } - - var errs field.ErrorList - - // Block all updates to Spec after creation - if !apiequality.Semantic.DeepEqual(key.Spec, oldKey.Spec) { - errs = append(errs, field.Forbidden( - field.NewPath("spec"), - "spec is immutable after creation", - )) - } - - return errs -} - -// Canonicalize normalizes the object. No-op here. -func (machineAccountKeyStrategy) Canonicalize(obj runtime.Object) {} - -// AllowCreateOnUpdate returns false — callers must use POST to create. -func (machineAccountKeyStrategy) AllowCreateOnUpdate() bool { return false } - -// AllowUnconditionalUpdate returns false — updates require a resourceVersion precondition. -func (machineAccountKeyStrategy) AllowUnconditionalUpdate() bool { return false } - -// WarningsOnCreate returns no warnings for MachineAccountKey creation. -func (machineAccountKeyStrategy) WarningsOnCreate(_ context.Context, _ runtime.Object) []string { - return nil -} - -// WarningsOnUpdate returns no warnings for MachineAccountKey updates. -func (machineAccountKeyStrategy) WarningsOnUpdate(_ context.Context, _, _ runtime.Object) []string { - return nil -} diff --git a/internal/apiserver/storage/identity/storageprovider.go b/internal/apiserver/storage/identity/storageprovider.go index fcd45214..90544896 100644 --- a/internal/apiserver/storage/identity/storageprovider.go +++ b/internal/apiserver/storage/identity/storageprovider.go @@ -2,9 +2,6 @@ package identity import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/runtime/serializer" generic "k8s.io/apiserver/pkg/registry/generic" "k8s.io/apiserver/pkg/registry/rest" genericapiserver "k8s.io/apiserver/pkg/server" @@ -15,20 +12,20 @@ import ( machineaccountkeysregistry "go.miloapis.com/milo/internal/apiserver/identity/machineaccountkeys" sessionsregistry "go.miloapis.com/milo/internal/apiserver/identity/sessions" useridentitiesregistry "go.miloapis.com/milo/internal/apiserver/identity/useridentities" - identityapi "go.miloapis.com/milo/pkg/apis/identity" identityv1alpha1 "go.miloapis.com/milo/pkg/apis/identity/v1alpha1" ) type StorageProvider struct { - Sessions sessionsregistry.Backend - UserIdentities useridentitiesregistry.Backend + Sessions sessionsregistry.Backend + UserIdentities useridentitiesregistry.Backend + MachineAccountKeys machineaccountkeysregistry.Backend } func (p StorageProvider) GroupName() string { return identityv1alpha1.SchemeGroupVersion.Group } func (p StorageProvider) NewRESTStorage( _ serverstorage.APIResourceConfigSource, - getter generic.RESTOptionsGetter, + _ generic.RESTOptionsGetter, ) (genericapiserver.APIGroupInfo, error) { apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo( identityv1alpha1.SchemeGroupVersion.Group, @@ -37,18 +34,10 @@ func (p StorageProvider) NewRESTStorage( legacyscheme.Codecs, ) - // Identity types do not have protobuf support, so we wrap the getter to override the - // storage codec with a JSON-only one. Without this, CompleteWithOptions fails with - // "internal type not encodable: object does not implement the protobuf marshalling interface". - machineAccountKeyStorage, err := machineaccountkeysregistry.NewREST(jsonOnlyGetter(getter)) - if err != nil { - return apiGroupInfo, err - } - storage := map[string]rest.Storage{ "sessions": sessionsregistry.NewREST(p.Sessions), "useridentities": useridentitiesregistry.NewREST(p.UserIdentities), - "machineaccountkeys": machineAccountKeyStorage, + "machineaccountkeys": machineaccountkeysregistry.NewREST(p.MachineAccountKeys), } apiGroupInfo.VersionedResourcesStorageMap = map[string]map[string]rest.Storage{ @@ -59,35 +48,3 @@ func (p StorageProvider) NewRESTStorage( } var _ controlplaneapiserver.RESTStorageProvider = StorageProvider{} - -// jsonOnlyGetter wraps a RESTOptionsGetter to override the storage codec with a -// JSON-only variant for identity resources. This is required because identity types -// do not implement the protobuf marshalling interface, and the default storage factory -// uses protobuf as the preferred encoding format. -type jsonOnlyGetterWrapper struct { - inner generic.RESTOptionsGetter - codec runtime.Codec -} - -func jsonOnlyGetter(inner generic.RESTOptionsGetter) generic.RESTOptionsGetter { - // Build a fresh scheme with only JSON serializers (no protobuf registered). - identityScheme := runtime.NewScheme() - identityapi.Install(identityScheme) - metav1.AddToGroupVersion(identityScheme, schema.GroupVersion{Group: "", Version: "v1"}) - - // serializer.NewCodecFactory registers JSON and YAML only — no protobuf. - identityCodecs := serializer.NewCodecFactory(identityScheme) - jsonCodec := identityCodecs.LegacyCodec(identityv1alpha1.SchemeGroupVersion) - - return &jsonOnlyGetterWrapper{inner: inner, codec: jsonCodec} -} - -func (g *jsonOnlyGetterWrapper) GetRESTOptions(gr schema.GroupResource, example runtime.Object) (generic.RESTOptions, error) { - opts, err := g.inner.GetRESTOptions(gr, example) - if err != nil { - return opts, err - } - // Replace the default codec (which tries protobuf) with a JSON-only one. - opts.StorageConfig.Codec = g.codec - return opts, nil -} diff --git a/pkg/apis/identity/v1alpha1/zz_generated.openapi.go b/pkg/apis/identity/v1alpha1/zz_generated.openapi.go index f216ad83..f473a06e 100644 --- a/pkg/apis/identity/v1alpha1/zz_generated.openapi.go +++ b/pkg/apis/identity/v1alpha1/zz_generated.openapi.go @@ -130,9 +130,9 @@ func schema_pkg_apis_identity_v1alpha1_MachineAccountKeySpec(ref common.Referenc Description: "MachineAccountKeySpec defines the desired state of MachineAccountKey", Type: []string{"object"}, Properties: map[string]spec.Schema{ - "machineAccountName": { + "machineAccountUserName": { SchemaProps: spec.SchemaProps{ - Description: "MachineAccountName is the name of the MachineAccount that owns this key.", + Description: "MachineAccountUserName is the email address of the MachineAccount that owns this key.", Default: "", Type: []string{"string"}, Format: "", @@ -152,7 +152,7 @@ func schema_pkg_apis_identity_v1alpha1_MachineAccountKeySpec(ref common.Referenc }, }, }, - Required: []string{"machineAccountName"}, + Required: []string{"machineAccountUserName"}, }, }, Dependencies: []string{ diff --git a/pkg/features/features.go b/pkg/features/features.go index 89bf6d65..af694a3b 100644 --- a/pkg/features/features.go +++ b/pkg/features/features.go @@ -47,6 +47,13 @@ const ( // alpha: v0.1.0 // ga: v0.2.0 UserIdentities featuregate.Feature = "UserIdentities" + + // MachineAccountKeys enables the identity.miloapis.com/v1alpha1 MachineAccountKey + // virtual API that proxies to an external identity provider for machine account key management. + // + // owner: @datum-cloud/platform + // alpha: v0.1.0 + MachineAccountKeys featuregate.Feature = "MachineAccountKeys" ) func init() { @@ -60,6 +67,10 @@ var defaultFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ Default: false, PreRelease: featuregate.Alpha, }, + MachineAccountKeys: { + Default: true, + PreRelease: featuregate.Alpha, + }, Sessions: { Default: true, PreRelease: featuregate.GA, From 1f18683c25b49b3b60abadde5631339bdb08cdeb Mon Sep 17 00:00:00 2001 From: Jose Szychowski Date: Tue, 31 Mar 2026 19:39:44 -0300 Subject: [PATCH 09/20] chore: format code --- cmd/milo/apiserver/server.go | 46 ++++++++++++------------- internal/quota/admission/plugin.go | 1 - internal/quota/admission/plugin_test.go | 6 ++-- 3 files changed, 26 insertions(+), 27 deletions(-) diff --git a/cmd/milo/apiserver/server.go b/cmd/milo/apiserver/server.go index 542892d0..3c165129 100644 --- a/cmd/milo/apiserver/server.go +++ b/cmd/milo/apiserver/server.go @@ -56,29 +56,29 @@ func init() { } var ( - SystemNamespace string - sessionsProviderURL string - sessionsProviderCAFile string - sessionsProviderClientCert string - sessionsProviderClientKey string - providerTimeoutSeconds int - providerRetries int - forwardExtras []string - userIdentitiesProviderURL string - userIdentitiesProviderCAFile string - userIdentitiesProviderClientCert string - userIdentitiesProviderClientKey string - machineAccountKeysProviderURL string - machineAccountKeysProviderCAFile string - machineAccountKeysProviderClientCert string - machineAccountKeysProviderClientKey string - eventsProviderURL string - eventsProviderCAFile string - eventsProviderClientCert string - eventsProviderClientKey string - eventsProviderTimeoutSeconds int - eventsProviderRetries int - eventsForwardExtras []string + SystemNamespace string + sessionsProviderURL string + sessionsProviderCAFile string + sessionsProviderClientCert string + sessionsProviderClientKey string + providerTimeoutSeconds int + providerRetries int + forwardExtras []string + userIdentitiesProviderURL string + userIdentitiesProviderCAFile string + userIdentitiesProviderClientCert string + userIdentitiesProviderClientKey string + machineAccountKeysProviderURL string + machineAccountKeysProviderCAFile string + machineAccountKeysProviderClientCert string + machineAccountKeysProviderClientKey string + eventsProviderURL string + eventsProviderCAFile string + eventsProviderClientCert string + eventsProviderClientKey string + eventsProviderTimeoutSeconds int + eventsProviderRetries int + eventsForwardExtras []string ) // NewCommand creates a *cobra.Command object with default parameters diff --git a/internal/quota/admission/plugin.go b/internal/quota/admission/plugin.go index e6ad472c..91c380a0 100644 --- a/internal/quota/admission/plugin.go +++ b/internal/quota/admission/plugin.go @@ -54,7 +54,6 @@ var ( }, []string{"result", "policy_name", "policy_namespace", "resource_group", "resource_kind"}, ) - ) func init() { diff --git a/internal/quota/admission/plugin_test.go b/internal/quota/admission/plugin_test.go index 68111b70..779ac4a1 100644 --- a/internal/quota/admission/plugin_test.go +++ b/internal/quota/admission/plugin_test.go @@ -1413,9 +1413,9 @@ func TestStructuredEndpointSliceCELTemplateRendering(t *testing.T) { plugin.watchManagers.Store("", &testWatchManager{behavior: "grant"}) tests := []struct { - name string - epsName string - object runtime.Object + name string + epsName string + object runtime.Object }{ { name: "structured discoveryv1.EndpointSlice", From 6d9ce04dd4a690d814356811db71acee0181a0d5 Mon Sep 17 00:00:00 2001 From: Jose Szychowski Date: Tue, 31 Mar 2026 19:43:54 -0300 Subject: [PATCH 10/20] chore: remove obsolete machine-account-key-creation chainsaw tests --- .../assertions/assert-key-auto-generated.yaml | 7 - .../assertions/assert-key-provided.yaml | 18 -- .../chainsaw-test.yaml | 217 ------------------ .../resources/key-auto-generate.yaml | 9 - .../resources/key-invalid-expiration.yaml | 8 - .../resources/key-malformed.yaml | 11 - .../resources/key-provided-public.yaml | 17 -- .../resources/machine-account.yaml | 7 - .../resources/organization.yaml | 11 - .../resources/project.yaml | 9 - 10 files changed, 314 deletions(-) delete mode 100644 test/identity/machine-account-key-creation/assertions/assert-key-auto-generated.yaml delete mode 100644 test/identity/machine-account-key-creation/assertions/assert-key-provided.yaml delete mode 100644 test/identity/machine-account-key-creation/chainsaw-test.yaml delete mode 100644 test/identity/machine-account-key-creation/resources/key-auto-generate.yaml delete mode 100644 test/identity/machine-account-key-creation/resources/key-invalid-expiration.yaml delete mode 100644 test/identity/machine-account-key-creation/resources/key-malformed.yaml delete mode 100644 test/identity/machine-account-key-creation/resources/key-provided-public.yaml delete mode 100644 test/identity/machine-account-key-creation/resources/machine-account.yaml delete mode 100644 test/identity/machine-account-key-creation/resources/organization.yaml delete mode 100644 test/identity/machine-account-key-creation/resources/project.yaml diff --git a/test/identity/machine-account-key-creation/assertions/assert-key-auto-generated.yaml b/test/identity/machine-account-key-creation/assertions/assert-key-auto-generated.yaml deleted file mode 100644 index fe9445a9..00000000 --- a/test/identity/machine-account-key-creation/assertions/assert-key-auto-generated.yaml +++ /dev/null @@ -1,7 +0,0 @@ -apiVersion: identity.miloapis.com/v1alpha1 -kind: MachineAccountKey -metadata: - name: test-key-auto -spec: - machineAccountName: test-machine-account - expirationDate: "2027-12-31T23:59:59Z" diff --git a/test/identity/machine-account-key-creation/assertions/assert-key-provided.yaml b/test/identity/machine-account-key-creation/assertions/assert-key-provided.yaml deleted file mode 100644 index 5ed914e7..00000000 --- a/test/identity/machine-account-key-creation/assertions/assert-key-provided.yaml +++ /dev/null @@ -1,18 +0,0 @@ -apiVersion: identity.miloapis.com/v1alpha1 -kind: MachineAccountKey -metadata: - name: test-key-provided -spec: - machineAccountName: test-machine-account - # Should match the provided key - publicKey: | - -----BEGIN PUBLIC KEY----- - MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA1hRF7sbBHvQ8E/vQXPxq - fg8qc3yGR6D75cv6xJwr82QU/twD0eGxVyzRV9Ndhq4DJNdDbKA5rseO36GiGP99 - S5uIUHi+BREdzsuBm+L8K2T5GcE6+5lPmlMjV12qVVJbyWoIEkse/k/zowjSmg5c - Qszc/lDglmWjFfNJG1YtmKqWvUXMNayV5whrpzygMTlHUes7GcGlfQ5gsXKudyYd - HSSeh3bXYfv6IdFj8UoJbFqfIjJNj7Dv4tyF5H7ShQKgSIrXDxzZqtuNLgImmoyE - gFL3FfS1mYO/lNNRctBhN/BkIrAB3bls0JDmJogBBbg9ttyE5QwsRXtV2LIof7Bm - PQIDAQAB - -----END PUBLIC KEY----- - expirationDate: "2027-12-31T23:59:59Z" diff --git a/test/identity/machine-account-key-creation/chainsaw-test.yaml b/test/identity/machine-account-key-creation/chainsaw-test.yaml deleted file mode 100644 index af79fffa..00000000 --- a/test/identity/machine-account-key-creation/chainsaw-test.yaml +++ /dev/null @@ -1,217 +0,0 @@ -apiVersion: chainsaw.kyverno.io/v1alpha1 -kind: Test -metadata: - name: machine-account-key-creation -spec: - description: | - Tests MachineAccountKey creation with automatic RSA key pair generation and validation. - - This test verifies: - - MachineAccountKey can be created without providing a public key (auto-generation) - - Auto-generated RSA public key is stored in spec.publicKey - - Private key is returned in creation response but NOT persisted to etcd - - MachineAccountKey can be created with a provided public key - - No private key is returned when a public key is provided - - Validation rejects expiration dates in the past - - Validation rejects malformed public keys - - Validation rejects any updates to spec after creation (immutability) - - Created resources are reconcilable (Ready condition initialized to Unknown) - - steps: - # Setup: create machine account in test namespace - - name: create-machine-account - description: Create a MachineAccount resource for testing keys - try: - - apply: - file: resources/machine-account.yaml - - # Test 1: Auto-generate RSA key pair - - name: test-auto-generate-key - description: Create MachineAccountKey without publicKey → auto-generate RSA pair - try: - # Create the resource and capture the raw response - - script: - timeout: 10s - content: | - # Create the resource and capture the raw response - kubectl apply -f resources/key-auto-generate.yaml -n $NAMESPACE -o json > /tmp/response.json - - # Verify that the response contains a privateKey in status - PRIVATE_KEY=$(jq -r '.status.privateKey // empty' /tmp/response.json) - - if [ -z "$PRIVATE_KEY" ]; then - echo "FAIL: privateKey not found in creation response" - echo "RAW RESPONSE:" - cat /tmp/response.json - exit 1 - fi - - # Verify the privateKey looks like a PEM-encoded RSA key - if ! echo "$PRIVATE_KEY" | grep -q "BEGIN RSA PRIVATE KEY"; then - echo "FAIL: privateKey is not PEM-encoded PKCS1" - exit 1 - fi - - # Verify publicKey was auto-generated in the response - PUBLIC_KEY=$(jq -r '.spec.publicKey // empty' /tmp/response.json) - if [ -z "$PUBLIC_KEY" ]; then - echo "FAIL: publicKey not found in creation response" - exit 1 - fi - - if ! echo "$PUBLIC_KEY" | grep -q "BEGIN PUBLIC KEY"; then - echo "FAIL: publicKey is not PEM-encoded PKIX" - exit 1 - fi - - echo "PASS: privateKey returned in creation response, publicKey auto-generated" - - # Verify the created key exists and is reconcilable - - wait: - apiVersion: identity.miloapis.com/v1alpha1 - kind: MachineAccountKey - name: test-key-auto - timeout: 10s - for: - condition: - name: Ready - value: Unknown - - # Assert the persisted object has correct structure - - assert: - file: assertions/assert-key-auto-generated.yaml - - # Verify privateKey is absent from GET response (not persisted to etcd) - - script: - timeout: 5s - content: | - # Fetch the resource from etcd - kubectl get machineaccountkey test-key-auto -n $NAMESPACE -o json > /tmp/fetched.json - - # Verify privateKey is NOT present - PRIVATE_KEY=$(jq -r '.status.privateKey // empty' /tmp/fetched.json) - - if [ -n "$PRIVATE_KEY" ]; then - echo "FAIL: privateKey should not be stored in etcd" - exit 1 - fi - - echo "PASS: privateKey correctly absent from persisted object" - - # Test 2: Provided public key (no auto-generation) - - name: test-provided-public-key - description: Create MachineAccountKey with publicKey → no auto-generation, no privateKey returned - try: - - script: - timeout: 10s - content: | - # Create the resource with provided public key - kubectl apply -f resources/key-provided-public.yaml -n $NAMESPACE -o json > /tmp/provided_response.json - - # Verify that the response does NOT contain a privateKey - PRIVATE_KEY=$(jq -r '.status.privateKey // empty' /tmp/provided_response.json) - - if [ -n "$PRIVATE_KEY" ]; then - echo "FAIL: privateKey should not be returned when publicKey is provided" - exit 1 - fi - - # Verify the publicKey is present - PUBLIC_KEY=$(jq -r '.spec.publicKey' /tmp/provided_response.json) - if [ -z "$PUBLIC_KEY" ]; then echo "FAIL: publicKey missing"; exit 1; fi - - echo "PASS: no privateKey returned with provided publicKey" - - - wait: - apiVersion: identity.miloapis.com/v1alpha1 - kind: MachineAccountKey - name: test-key-provided - timeout: 10s - for: - condition: - name: Ready - value: Unknown - # Assert the public key matches what was provided - - assert: - file: assertions/assert-key-provided.yaml - - # Test 3: Validation - reject expiration date in past - - name: test-validation-expiration-past - description: Reject MachineAccountKey with expirationDate in the past - try: - - script: - timeout: 10s - content: | - if kubectl apply -f resources/key-invalid-expiration.yaml -n $NAMESPACE 2> /tmp/val_err.json; then - echo "FAIL: resource should have been rejected" - exit 1 - fi - - if grep -q "expirationDate must be in the future" /tmp/val_err.json; then - echo "PASS: expirationDate validation triggered" - else - echo "FAIL: wrong error message" - cat /tmp/val_err.json - exit 1 - fi - - # Test 5: Immutability - reject any spec update - - name: test-spec-immutability - description: Reject any update to MachineAccountKey.spec after creation - try: - - script: - timeout: 10s - content: | - # Attempt to patch the expirationDate - if kubectl patch machineaccountkey test-key-auto -n $NAMESPACE --type=merge -p '{"spec":{"expirationDate":"2030-01-01T00:00:00Z"}}' 2> /tmp/immutability_err.json; then - echo "FAIL: spec update should have been rejected" - exit 1 - fi - - if grep -q "spec is immutable after creation" /tmp/immutability_err.json; then - echo "PASS: spec immutability validation triggered" - else - echo "FAIL: wrong error message" - cat /tmp/immutability_err.json - exit 1 - fi - - # Test 6: Validation - reject malformed public key - - name: test-validation-malformed-key - description: Reject MachineAccountKey with malformed publicKey - try: - - script: - timeout: 10s - content: | - if kubectl apply -f resources/key-malformed.yaml -n $NAMESPACE 2> /tmp/malformed_err.json; then - echo "FAIL: malformed key should have been rejected" - exit 1 - fi - - if grep -q "publicKey" /tmp/malformed_err.json; then - echo "PASS: malformed key validation triggered" - else - echo "FAIL: wrong error message" - cat /tmp/malformed_err.json - exit 1 - fi - - # Test 7: Verify resource schema and conditions - - name: test-resource-conditions - description: Verify MachineAccountKey initializes with correct conditions - try: - - script: - timeout: 5s - content: | - READY_STATUS=$(kubectl get machineaccountkey test-key-auto -n $NAMESPACE -o json | \ - jq -r '.status.conditions[] | select(.type=="Ready") | .status') - - if [ "$READY_STATUS" != "Unknown" ]; then - echo "FAIL: Expected Ready condition to be Unknown, got $READY_STATUS" - exit 1 - fi - - echo "PASS: conditions are correctly initialized" - - - diff --git a/test/identity/machine-account-key-creation/resources/key-auto-generate.yaml b/test/identity/machine-account-key-creation/resources/key-auto-generate.yaml deleted file mode 100644 index 81e59539..00000000 --- a/test/identity/machine-account-key-creation/resources/key-auto-generate.yaml +++ /dev/null @@ -1,9 +0,0 @@ -apiVersion: identity.miloapis.com/v1alpha1 -kind: MachineAccountKey -metadata: - name: test-key-auto - -spec: - machineAccountName: test-machine-account - expirationDate: "2027-12-31T23:59:59Z" - publicKey: "" diff --git a/test/identity/machine-account-key-creation/resources/key-invalid-expiration.yaml b/test/identity/machine-account-key-creation/resources/key-invalid-expiration.yaml deleted file mode 100644 index 10618fbb..00000000 --- a/test/identity/machine-account-key-creation/resources/key-invalid-expiration.yaml +++ /dev/null @@ -1,8 +0,0 @@ -apiVersion: identity.miloapis.com/v1alpha1 -kind: MachineAccountKey -metadata: - name: test-key-invalid-exp - -spec: - machineAccountName: test-machine-account - expirationDate: "2020-01-01T00:00:00Z" diff --git a/test/identity/machine-account-key-creation/resources/key-malformed.yaml b/test/identity/machine-account-key-creation/resources/key-malformed.yaml deleted file mode 100644 index 109eae90..00000000 --- a/test/identity/machine-account-key-creation/resources/key-malformed.yaml +++ /dev/null @@ -1,11 +0,0 @@ -apiVersion: identity.miloapis.com/v1alpha1 -kind: MachineAccountKey -metadata: - name: test-key-malformed - -spec: - machineAccountName: example-machine-account - publicKey: | - -----BEGIN PUBLIC KEY----- - THIS-IS-NOT-A-KEY - -----END PUBLIC KEY----- diff --git a/test/identity/machine-account-key-creation/resources/key-provided-public.yaml b/test/identity/machine-account-key-creation/resources/key-provided-public.yaml deleted file mode 100644 index 12e22f32..00000000 --- a/test/identity/machine-account-key-creation/resources/key-provided-public.yaml +++ /dev/null @@ -1,17 +0,0 @@ -apiVersion: identity.miloapis.com/v1alpha1 -kind: MachineAccountKey -metadata: - name: test-key-provided -spec: - machineAccountName: test-machine-account - publicKey: | - -----BEGIN PUBLIC KEY----- - MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA1hRF7sbBHvQ8E/vQXPxq - fg8qc3yGR6D75cv6xJwr82QU/twD0eGxVyzRV9Ndhq4DJNdDbKA5rseO36GiGP99 - S5uIUHi+BREdzsuBm+L8K2T5GcE6+5lPmlMjV12qVVJbyWoIEkse/k/zowjSmg5c - Qszc/lDglmWjFfNJG1YtmKqWvUXMNayV5whrpzygMTlHUes7GcGlfQ5gsXKudyYd - HSSeh3bXYfv6IdFj8UoJbFqfIjJNj7Dv4tyF5H7ShQKgSIrXDxzZqtuNLgImmoyE - gFL3FfS1mYO/lNNRctBhN/BkIrAB3bls0JDmJogBBbg9ttyE5QwsRXtV2LIof7Bm - PQIDAQAB - -----END PUBLIC KEY----- - expirationDate: "2027-12-31T23:59:59Z" diff --git a/test/identity/machine-account-key-creation/resources/machine-account.yaml b/test/identity/machine-account-key-creation/resources/machine-account.yaml deleted file mode 100644 index ec68c7f1..00000000 --- a/test/identity/machine-account-key-creation/resources/machine-account.yaml +++ /dev/null @@ -1,7 +0,0 @@ -apiVersion: iam.miloapis.com/v1alpha1 -kind: MachineAccount -metadata: - name: test-machine-account - -spec: - state: Active diff --git a/test/identity/machine-account-key-creation/resources/organization.yaml b/test/identity/machine-account-key-creation/resources/organization.yaml deleted file mode 100644 index b8e93387..00000000 --- a/test/identity/machine-account-key-creation/resources/organization.yaml +++ /dev/null @@ -1,11 +0,0 @@ -apiVersion: resourcemanager.miloapis.com/v1alpha1 -kind: Organization -metadata: - name: mak-test-org -spec: - type: Standard ---- -apiVersion: v1 -kind: Namespace -metadata: - name: organization-mak-test-org diff --git a/test/identity/machine-account-key-creation/resources/project.yaml b/test/identity/machine-account-key-creation/resources/project.yaml deleted file mode 100644 index 69a12f18..00000000 --- a/test/identity/machine-account-key-creation/resources/project.yaml +++ /dev/null @@ -1,9 +0,0 @@ -apiVersion: resourcemanager.miloapis.com/v1alpha1 -kind: Project -metadata: - name: mak-test-project - namespace: organization-mak-test-org -spec: - ownerRef: - kind: Organization - name: mak-test-org From db471ce9a139108b77c3f457a4814cd6a0fd750e Mon Sep 17 00:00:00 2001 From: Jose Szychowski Date: Tue, 31 Mar 2026 20:37:44 -0300 Subject: [PATCH 11/20] feat: add identity-machine-account-keys-admin role to project-admin configuration --- config/roles/project-admin.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/config/roles/project-admin.yaml b/config/roles/project-admin.yaml index 15b398ec..ad94aff2 100644 --- a/config/roles/project-admin.yaml +++ b/config/roles/project-admin.yaml @@ -9,6 +9,7 @@ spec: launchStage: Beta inheritedRoles: - name: resourcemanager.miloapis.com-project-viewer + - name: identity-machine-account-keys-admin includedPermissions: - resourcemanager.miloapis.com/projects.create - resourcemanager.miloapis.com/projects.update From a552f5e0dda73995a095e9c3d533ecbf4ac5d4e9 Mon Sep 17 00:00:00 2001 From: Jose Szychowski Date: Tue, 31 Mar 2026 21:23:51 -0300 Subject: [PATCH 12/20] feat: add project key to forward-extras configuration --- cmd/milo/apiserver/server.go | 2 +- config/apiserver/deployment.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/milo/apiserver/server.go b/cmd/milo/apiserver/server.go index 3c165129..4143b95a 100644 --- a/cmd/milo/apiserver/server.go +++ b/cmd/milo/apiserver/server.go @@ -183,7 +183,7 @@ func NewCommand() *cobra.Command { fs.StringVar(&sessionsProviderClientKey, "sessions-provider-client-key", "", "Client private key for mTLS to provider") fs.IntVar(&providerTimeoutSeconds, "provider-timeout", 3, "Provider request timeout in seconds") fs.IntVar(&providerRetries, "provider-retries", 2, "Provider request retries") - fs.StringSliceVar(&forwardExtras, "forward-extras", []string{"iam.miloapis.com/parent-api-group", "iam.miloapis.com/parent-type", "iam.miloapis.com/parent-name"}, "User extras keys to forward during impersonation") + fs.StringSliceVar(&forwardExtras, "forward-extras", []string{"iam.miloapis.com/parent-api-group", "iam.miloapis.com/parent-type", "iam.miloapis.com/parent-name", "project"}, "User extras keys to forward during impersonation") fs.StringVar(&userIdentitiesProviderURL, "useridentities-provider-url", "", "Direct provider base URL for useridentities (e.g., https://zitadel-apiserver:8443)") fs.StringVar(&userIdentitiesProviderCAFile, "useridentities-provider-ca-file", "", "Path to CA file to validate useridentities provider TLS") fs.StringVar(&userIdentitiesProviderClientCert, "useridentities-provider-client-cert", "", "Client certificate for mTLS to useridentities provider") diff --git a/config/apiserver/deployment.yaml b/config/apiserver/deployment.yaml index e13dd69f..84473490 100644 --- a/config/apiserver/deployment.yaml +++ b/config/apiserver/deployment.yaml @@ -64,7 +64,7 @@ spec: - --sessions-provider-client-key=$(SESSIONS_PROVIDER_CLIENT_KEY_FILE) - --provider-timeout=3 - --provider-retries=2 - - --forward-extras=iam.miloapis.com/parent-api-group,iam.miloapis.com/parent-type,iam.miloapis.com/parent-name + - --forward-extras=iam.miloapis.com/parent-api-group,iam.miloapis.com/parent-type,iam.miloapis.com/parent-name,project # UserIdentities provider configuration - --useridentities-provider-url=$(USERIDENTITIES_PROVIDER_URL) - --useridentities-provider-ca-file=$(USERIDENTITIES_PROVIDER_CA_FILE) From 2574c6f1a9e7010824f69e5dc69867df52c0dfb9 Mon Sep 17 00:00:00 2001 From: Jose Szychowski Date: Tue, 31 Mar 2026 22:42:04 -0300 Subject: [PATCH 13/20] feat: add field selector support for MachineAccountKey --- pkg/apis/identity/scheme.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/pkg/apis/identity/scheme.go b/pkg/apis/identity/scheme.go index a6714d59..8f4e8878 100644 --- a/pkg/apis/identity/scheme.go +++ b/pkg/apis/identity/scheme.go @@ -2,6 +2,7 @@ package identity import ( "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "go.miloapis.com/milo/pkg/apis/identity/v1alpha1" ) @@ -9,4 +10,22 @@ import ( // Install registers the identity API group versions into the provided scheme. func Install(scheme *runtime.Scheme) { v1alpha1.AddToScheme(scheme) + + // Register valid field selectors for MachineAccountKey so the generic API + // server passes them through to the REST handler instead of rejecting them. + _ = scheme.AddFieldLabelConversionFunc( + schema.GroupVersionKind{ + Group: v1alpha1.SchemeGroupVersion.Group, + Version: v1alpha1.SchemeGroupVersion.Version, + Kind: "MachineAccountKey", + }, + func(label, value string) (string, string, error) { + switch label { + case "spec.machineAccountUserName", "metadata.name", "metadata.namespace": + return label, value, nil + default: + return "", "", nil + } + }, + ) } From 6f663813bc33a9e3e59cebfa33ee09b094c194a4 Mon Sep 17 00:00:00 2001 From: Jose Szychowski Date: Tue, 31 Mar 2026 23:45:48 -0300 Subject: [PATCH 14/20] Revert "feat: add project key to forward-extras configuration" This reverts commit a552f5e0dda73995a095e9c3d533ecbf4ac5d4e9. --- cmd/milo/apiserver/server.go | 2 +- config/apiserver/deployment.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/milo/apiserver/server.go b/cmd/milo/apiserver/server.go index 4143b95a..3c165129 100644 --- a/cmd/milo/apiserver/server.go +++ b/cmd/milo/apiserver/server.go @@ -183,7 +183,7 @@ func NewCommand() *cobra.Command { fs.StringVar(&sessionsProviderClientKey, "sessions-provider-client-key", "", "Client private key for mTLS to provider") fs.IntVar(&providerTimeoutSeconds, "provider-timeout", 3, "Provider request timeout in seconds") fs.IntVar(&providerRetries, "provider-retries", 2, "Provider request retries") - fs.StringSliceVar(&forwardExtras, "forward-extras", []string{"iam.miloapis.com/parent-api-group", "iam.miloapis.com/parent-type", "iam.miloapis.com/parent-name", "project"}, "User extras keys to forward during impersonation") + fs.StringSliceVar(&forwardExtras, "forward-extras", []string{"iam.miloapis.com/parent-api-group", "iam.miloapis.com/parent-type", "iam.miloapis.com/parent-name"}, "User extras keys to forward during impersonation") fs.StringVar(&userIdentitiesProviderURL, "useridentities-provider-url", "", "Direct provider base URL for useridentities (e.g., https://zitadel-apiserver:8443)") fs.StringVar(&userIdentitiesProviderCAFile, "useridentities-provider-ca-file", "", "Path to CA file to validate useridentities provider TLS") fs.StringVar(&userIdentitiesProviderClientCert, "useridentities-provider-client-cert", "", "Client certificate for mTLS to useridentities provider") diff --git a/config/apiserver/deployment.yaml b/config/apiserver/deployment.yaml index 84473490..e13dd69f 100644 --- a/config/apiserver/deployment.yaml +++ b/config/apiserver/deployment.yaml @@ -64,7 +64,7 @@ spec: - --sessions-provider-client-key=$(SESSIONS_PROVIDER_CLIENT_KEY_FILE) - --provider-timeout=3 - --provider-retries=2 - - --forward-extras=iam.miloapis.com/parent-api-group,iam.miloapis.com/parent-type,iam.miloapis.com/parent-name,project + - --forward-extras=iam.miloapis.com/parent-api-group,iam.miloapis.com/parent-type,iam.miloapis.com/parent-name # UserIdentities provider configuration - --useridentities-provider-url=$(USERIDENTITIES_PROVIDER_URL) - --useridentities-provider-ca-file=$(USERIDENTITIES_PROVIDER_CA_FILE) From 17b74e7729c58d882627b6bdbbd2375adf763535 Mon Sep 17 00:00:00 2001 From: Jose Szychowski Date: Tue, 31 Mar 2026 23:46:46 -0300 Subject: [PATCH 15/20] Revert "feat: add identity-machine-account-keys-admin role to project-admin configuration" This reverts commit db471ce9a139108b77c3f457a4814cd6a0fd750e. --- config/roles/project-admin.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/config/roles/project-admin.yaml b/config/roles/project-admin.yaml index ad94aff2..15b398ec 100644 --- a/config/roles/project-admin.yaml +++ b/config/roles/project-admin.yaml @@ -9,7 +9,6 @@ spec: launchStage: Beta inheritedRoles: - name: resourcemanager.miloapis.com-project-viewer - - name: identity-machine-account-keys-admin includedPermissions: - resourcemanager.miloapis.com/projects.create - resourcemanager.miloapis.com/projects.update From 012ff56e6eb94d6118bddfb2001ba910b0ad773b Mon Sep 17 00:00:00 2001 From: Jose Szychowski Date: Wed, 1 Apr 2026 14:14:16 -0300 Subject: [PATCH 16/20] chore: autogenerate code --- .../identity/v1alpha1/machineaccountkey.yaml | 2 +- docs/api/iam.md | 215 ----------------- docs/api/notes.md | 88 ++++--- docs/api/notification.md | 219 ++++++++++++++++++ test/crm/note-contact-lifecycle/README.md | 6 +- .../README.md | 78 +++++++ .../notes/note-multicluster-subject/README.md | 77 ++++++ .../structured-type-enforcement/README.md | 205 ++++++++++++++++ 8 files changed, 624 insertions(+), 266 deletions(-) create mode 100644 test/notes/clusternote-multicluster-subject/README.md create mode 100644 test/notes/note-multicluster-subject/README.md create mode 100644 test/quota/structured-type-enforcement/README.md diff --git a/config/samples/identity/v1alpha1/machineaccountkey.yaml b/config/samples/identity/v1alpha1/machineaccountkey.yaml index 8cf22d99..5d717fb0 100644 --- a/config/samples/identity/v1alpha1/machineaccountkey.yaml +++ b/config/samples/identity/v1alpha1/machineaccountkey.yaml @@ -4,7 +4,7 @@ metadata: name: example-machine-account-key-32 namespace: default spec: - machineAccountName: example-machine-account + machineAccountUserName: example-machine-account # If not specified, the key will never expire. # expirationDate: "2026-03-25T10:18:48Z" # If not specified, an auto-generated public key will be created. diff --git a/docs/api/iam.md b/docs/api/iam.md index 1ee17986..ccc43037 100644 --- a/docs/api/iam.md +++ b/docs/api/iam.md @@ -12,8 +12,6 @@ Resource Types: - [Group](#group) -- [MachineAccountKey](#machineaccountkey) - - [MachineAccount](#machineaccount) - [PlatformAccessApproval](#platformaccessapproval) @@ -376,219 +374,6 @@ GroupStatus defines the observed state of Group -Condition contains details for one aspect of the current state of this API Resource. - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
NameTypeDescriptionRequired
lastTransitionTimestring - lastTransitionTime is the last time the condition transitioned from one status to another. -This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable.
-
- Format: date-time
- 		true
messagestring - message is a human readable message indicating details about the transition. -This may be an empty string.
- 		true
reasonstring - reason contains a programmatic identifier indicating the reason for the condition's last transition. -Producers of specific condition types may define expected values and meanings for this field, -and whether the values are considered a guaranteed API. -The value should be a CamelCase string. -This field may not be empty.
- 		true
statusenum - status of the condition, one of True, False, Unknown.
-
- Enum: True, False, Unknown
- 		true
typestring - type of condition in CamelCase or in foo.example.com/CamelCase.
- 		true
observedGenerationinteger - observedGeneration represents the .metadata.generation that the condition was set based upon. -For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date -with respect to the current state of the instance.
-
- Format: int64
- Minimum: 0
- 		false
- -## MachineAccountKey -[↩ Parent](#iammiloapiscomv1alpha1 ) - - - - - - -MachineAccountKey is the Schema for the machineaccountkeys API - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
NameTypeDescriptionRequired
apiVersionstringiam.miloapis.com/v1alpha1true
kindstringMachineAccountKeytrue
metadataobjectRefer to the Kubernetes API documentation for the fields of the `metadata` field.true
specobject - MachineAccountKeySpec defines the desired state of MachineAccountKey
- 		false
statusobject - MachineAccountKeyStatus defines the observed state of MachineAccountKey
- 		false
- - -### MachineAccountKey.spec -[↩ Parent](#machineaccountkey) - - - -MachineAccountKeySpec defines the desired state of MachineAccountKey - - - - - - - - - - - - - - - - - - - - - - - - - - -
NameTypeDescriptionRequired
machineAccountNamestring - MachineAccountName is the name of the MachineAccount that owns this key.
- 		true
expirationDatestring - ExpirationDate is the date and time when the MachineAccountKey will expire. -If not specified, the MachineAccountKey will never expire.
-
- Format: date-time
- 		false
publicKeystring - PublicKey is the public key of the MachineAccountKey. -If not specified, the MachineAccountKey will be created with an auto-generated public key.
- 		false
- - -### MachineAccountKey.status -[↩ Parent](#machineaccountkey) - - - -MachineAccountKeyStatus defines the observed state of MachineAccountKey - - - - - - - - - - - - - - - - - - - - - -
NameTypeDescriptionRequired
authProviderKeyIdstring - AuthProviderKeyID is the unique identifier for the key in the auth provider. -This field is populated by the controller after the key is created in the auth provider. -For example, when using Zitadel, a typical value might be: "326102453042806786"
- 		false
conditions[]object - Conditions provide conditions that represent the current status of the MachineAccountKey.
-
- Default: [map[lastTransitionTime:1970-01-01T00:00:00Z message:Waiting for control plane to reconcile reason:Unknown status:Unknown type:Ready]]
- 		false
- - -### MachineAccountKey.status.conditions[index] -[↩ Parent](#machineaccountkeystatus) - - - Condition contains details for one aspect of the current state of this API Resource. diff --git a/docs/api/notes.md b/docs/api/notes.md index ba1faf9f..7247a2df 100644 --- a/docs/api/notes.md +++ b/docs/api/notes.md @@ -8,14 +8,14 @@ Packages: Resource Types: -- [Note](#note) - - [ClusterNote](#clusternote) +- [Note](#note) + -## Note +## ClusterNote [↩ Parent](#notesmiloapiscomv1alpha1 ) @@ -23,8 +23,8 @@ Resource Types: -Note is the Schema for the notes API. -It represents a namespaced note attached to a subject resource. +ClusterNote is the Schema for the cluster-scoped notes API. +It represents a note attached to a cluster-scoped subject resource.
@@ -44,7 +44,7 @@ It represents a namespaced note attached to a subject resource. - + @@ -53,14 +53,14 @@ It represents a namespaced note attached to a subject resource. - + - +
kind stringNoteClusterNote true
Refer to the Kubernetes API documentation for the fields of the `metadata` field. true
specspec object NoteSpec defines the desired state of Note.
 false
statusstatus object NoteStatus defines the observed state of Note
@@ -70,8 +70,8 @@ It represents a namespaced note attached to a subject resource.
-### Note.spec -[↩ Parent](#note) +### ClusterNote.spec +[↩ Parent](#clusternote) @@ -91,12 +91,10 @@ NoteSpec defines the desired state of Note. string Content is the text content of the note.
-
- Validations:
  • MaxLength: 1000
    • true - subjectRef + subjectRef object Subject is a reference to the subject of the note.
    @@ -105,7 +103,7 @@ NoteSpec defines the desired state of Note. true - creatorRef + creatorRef object CreatorRef is a reference to the user that created the note. @@ -113,7 +111,7 @@ Defaults to the user that created the note.

    Validations:
  • type(oldSelf) == null_type || self == oldSelf: creatorRef type is immutable
    • - true + false followUp boolean @@ -153,8 +151,8 @@ When true, the note is being actively tracked for further action.
    -### Note.spec.subjectRef -[↩ Parent](#notespec) +### ClusterNote.spec.subjectRef +[↩ Parent](#clusternotespec) @@ -202,8 +200,8 @@ Required for namespace-scoped resources. Omitted for cluster-scoped resources. -### Note.spec.creatorRef -[↩ Parent](#notespec) +### ClusterNote.spec.creatorRef +[↩ Parent](#clusternotespec) @@ -230,8 +228,8 @@ Defaults to the user that created the note. -### Note.status -[↩ Parent](#note) +### ClusterNote.status +[↩ Parent](#clusternote) @@ -247,7 +245,7 @@ NoteStatus defines the observed state of Note - conditions + conditions []object Conditions provide conditions that represent the current status of the Note.
    @@ -266,8 +264,8 @@ NoteStatus defines the observed state of Note -### Note.status.conditions[index] -[↩ Parent](#notestatus) +### ClusterNote.status.conditions[index] +[↩ Parent](#clusternotestatus) @@ -342,7 +340,7 @@ with respect to the current state of the instance.
    -## ClusterNote +## Note [↩ Parent](#notesmiloapiscomv1alpha1 ) @@ -350,8 +348,8 @@ with respect to the current state of the instance.
    -ClusterNote is the Schema for the cluster-scoped notes API. -It represents a note attached to a cluster-scoped subject resource. +Note is the Schema for the notes API. +It represents a namespaced note attached to a subject resource. @@ -371,7 +369,7 @@ It represents a note attached to a cluster-scoped subject resource. - + @@ -380,14 +378,14 @@ It represents a note attached to a cluster-scoped subject resource. - + - +
    kind stringClusterNoteNote true
    Refer to the Kubernetes API documentation for the fields of the `metadata` field. true
    specspec object NoteSpec defines the desired state of Note.
    		 false
    statusstatus object NoteStatus defines the observed state of Note
    @@ -397,8 +395,8 @@ It represents a note attached to a cluster-scoped subject resource.
    -### ClusterNote.spec -[↩ Parent](#clusternote) +### Note.spec +[↩ Parent](#note) @@ -418,12 +416,10 @@ NoteSpec defines the desired state of Note. string Content is the text content of the note.
    -
    - Validations:
  • MaxLength: 1000
    • true - subjectRef + subjectRef object Subject is a reference to the subject of the note.
    @@ -432,7 +428,7 @@ NoteSpec defines the desired state of Note. true - creatorRef + creatorRef object CreatorRef is a reference to the user that created the note. @@ -440,7 +436,7 @@ Defaults to the user that created the note.

    Validations:
  • type(oldSelf) == null_type || self == oldSelf: creatorRef type is immutable
    • - true + false followUp boolean @@ -480,8 +476,8 @@ When true, the note is being actively tracked for further action.
    -### ClusterNote.spec.subjectRef -[↩ Parent](#clusternotespec) +### Note.spec.subjectRef +[↩ Parent](#notespec) @@ -529,8 +525,8 @@ Required for namespace-scoped resources. Omitted for cluster-scoped resources. -### ClusterNote.spec.creatorRef -[↩ Parent](#clusternotespec) +### Note.spec.creatorRef +[↩ Parent](#notespec) @@ -557,8 +553,8 @@ Defaults to the user that created the note. -### ClusterNote.status -[↩ Parent](#clusternote) +### Note.status +[↩ Parent](#note) @@ -574,7 +570,7 @@ NoteStatus defines the observed state of Note - conditions + conditions []object Conditions provide conditions that represent the current status of the Note.
    @@ -593,8 +589,8 @@ NoteStatus defines the observed state of Note -### ClusterNote.status.conditions[index] -[↩ Parent](#clusternotestatus) +### Note.status.conditions[index] +[↩ Parent](#notestatus) diff --git a/docs/api/notification.md b/docs/api/notification.md index 8c1b1137..2c77299d 100644 --- a/docs/api/notification.md +++ b/docs/api/notification.md @@ -22,6 +22,8 @@ Resource Types: - [EmailTemplate](#emailtemplate) +- [Note](#note) + @@ -2208,3 +2210,220 @@ with respect to the current state of the instance.
    +## Note +[↩ Parent](#notificationmiloapiscomv1alpha1 ) + + + + + + +Note is the Schema for the notes API. +It represents a note attached to a contact. + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
    NameTypeDescriptionRequired
    apiVersionstringnotification.miloapis.com/v1alpha1true
    kindstringNotetrue
    metadataobjectRefer to the Kubernetes API documentation for the fields of the `metadata` field.true
    specobject + NoteSpec defines the desired state of Note.
    +     		false
    statusobject + NoteStatus defines the observed state of Note.
    +     		false
    + + +### Note.spec +[↩ Parent](#note) + + + +NoteSpec defines the desired state of Note. + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
    NameTypeDescriptionRequired
    contactRefstring + ContactRef is the name of the Contact this note is attached to.
    +     		true
    contentstring + Content is the text content of the note.
    +     		true
    actionstring + Action is an optional follow-up action.
    +     		false
    actionTimestring + ActionTime is the timestamp for the follow-up action.
    +
    + Format: date-time
    +     		false
    interactionTimestring + InteractionTime is the timestamp of the interaction. +If not specified, it defaults to the creation timestamp of the note.
    +
    + Format: date-time
    +     		false
    + + +### Note.status +[↩ Parent](#note) + + + +NoteStatus defines the observed state of Note. + + + + + + + + + + + + + + + + +
    NameTypeDescriptionRequired
    conditions[]object + Conditions represent the latest available observations of an object's state
    +     		false
    + + +### Note.status.conditions[index] +[↩ Parent](#notestatus) + + + +Condition contains details for one aspect of the current state of this API Resource. + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
    NameTypeDescriptionRequired
    lastTransitionTimestring + lastTransitionTime is the last time the condition transitioned from one status to another. +This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable.
    +
    + Format: date-time
    +     		true
    messagestring + message is a human readable message indicating details about the transition. +This may be an empty string.
    +     		true
    reasonstring + reason contains a programmatic identifier indicating the reason for the condition's last transition. +Producers of specific condition types may define expected values and meanings for this field, +and whether the values are considered a guaranteed API. +The value should be a CamelCase string. +This field may not be empty.
    +     		true
    statusenum + status of the condition, one of True, False, Unknown.
    +
    + Enum: True, False, Unknown
    +     		true
    typestring + type of condition in CamelCase or in foo.example.com/CamelCase.
    +     		true
    observedGenerationinteger + observedGeneration represents the .metadata.generation that the condition was set based upon. +For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date +with respect to the current state of the instance.
    +
    + Format: int64
    + Minimum: 0
    +     		false
    diff --git a/test/crm/note-contact-lifecycle/README.md b/test/crm/note-contact-lifecycle/README.md index 766d3a24..fe422110 100644 --- a/test/crm/note-contact-lifecycle/README.md +++ b/test/crm/note-contact-lifecycle/README.md @@ -12,7 +12,7 @@ This test verifies: | # | Name | Bindings | Try | Catch | Finally | Cleanup | |:-:|---|:-:|:-:|:-:|:-:|:-:| -| 1 | [create-user-contact-and-notes](#step-create-user-contact-and-notes) | 0 | 7 | 0 | 0 | 0 | +| 1 | [create-user-contact-and-notes](#step-create-user-contact-and-notes) | 0 | 5 | 0 | 0 | 0 | | 2 | [delete-contact-and-verify-contact-note-deletion](#step-delete-contact-and-verify-contact-note-deletion) | 0 | 5 | 0 | 0 | 0 | | 3 | [delete-user-and-verify-user-note-deletion](#step-delete-user-and-verify-user-note-deletion) | 0 | 3 | 0 | 0 | 0 | | 4 | [verify-additional-notes-still-exist](#step-verify-additional-notes-still-exist) | 0 | 2 | 0 | 0 | 0 | @@ -29,9 +29,7 @@ Create IAM User, Notification Contact, and CRM Notes, then verify Note status | 2 | `wait` | 0 | 0 | *No description* | | 3 | `apply` | 0 | 0 | *No description* | | 4 | `apply` | 0 | 0 | *No description* | -| 5 | `script` | 0 | 0 | *No description* | -| 6 | `wait` | 0 | 0 | *No description* | -| 7 | `wait` | 0 | 0 | *No description* | +| 5 | `assert` | 0 | 0 | *No description* | ### Step: `delete-contact-and-verify-contact-note-deletion` diff --git a/test/notes/clusternote-multicluster-subject/README.md b/test/notes/clusternote-multicluster-subject/README.md new file mode 100644 index 00000000..ccb91295 --- /dev/null +++ b/test/notes/clusternote-multicluster-subject/README.md @@ -0,0 +1,78 @@ +# Test: `clusternote-multicluster-subject` + +Tests that ClusterNotes can reference cluster-scoped subjects (Namespaces) +in project control planes. + +Validates: +- ClusterNote in project control plane can reference cluster-scoped Namespace +- Owner reference is correctly set on the ClusterNote +- ClusterNote is garbage collected when Namespace is deleted + + +## Steps + +| # | Name | Bindings | Try | Catch | Finally | Cleanup | +|:-:|---|:-:|:-:|:-:|:-:|:-:| +| 1 | [setup-organization](#step-setup-organization) | 0 | 2 | 0 | 0 | 0 | +| 2 | [create-project](#step-create-project) | 0 | 2 | 0 | 0 | 0 | +| 3 | [create-namespace-in-project](#step-create-namespace-in-project) | 0 | 2 | 0 | 0 | 0 | +| 4 | [create-clusternote-referencing-namespace](#step-create-clusternote-referencing-namespace) | 0 | 2 | 0 | 0 | 0 | +| 5 | [delete-namespace-verify-clusternote-deletion](#step-delete-namespace-verify-clusternote-deletion) | 0 | 2 | 0 | 0 | 0 | + +### Step: `setup-organization` + +Create test organization + +#### Try + +| # | Operation | Bindings | Outputs | Description | +|:-:|---|:-:|:-:|---| +| 1 | `apply` | 0 | 0 | *No description* | +| 2 | `wait` | 0 | 0 | *No description* | + +### Step: `create-project` + +Create project in org control plane + +#### Try + +| # | Operation | Bindings | Outputs | Description | +|:-:|---|:-:|:-:|---| +| 1 | `apply` | 0 | 0 | *No description* | +| 2 | `wait` | 0 | 0 | *No description* | + +### Step: `create-namespace-in-project` + +Create cluster-scoped Namespace in project control plane + +#### Try + +| # | Operation | Bindings | Outputs | Description | +|:-:|---|:-:|:-:|---| +| 1 | `apply` | 0 | 0 | *No description* | +| 2 | `wait` | 0 | 0 | *No description* | + +### Step: `create-clusternote-referencing-namespace` + +Create ClusterNote referencing the Namespace in project control plane + +#### Try + +| # | Operation | Bindings | Outputs | Description | +|:-:|---|:-:|:-:|---| +| 1 | `apply` | 0 | 0 | *No description* | +| 2 | `assert` | 0 | 0 | *No description* | + +### Step: `delete-namespace-verify-clusternote-deletion` + +Delete Namespace and verify ClusterNote is garbage collected + +#### Try + +| # | Operation | Bindings | Outputs | Description | +|:-:|---|:-:|:-:|---| +| 1 | `delete` | 0 | 0 | *No description* | +| 2 | `wait` | 0 | 0 | *No description* | + +--- + diff --git a/test/notes/note-multicluster-subject/README.md b/test/notes/note-multicluster-subject/README.md new file mode 100644 index 00000000..06ca26d3 --- /dev/null +++ b/test/notes/note-multicluster-subject/README.md @@ -0,0 +1,77 @@ +# Test: `note-multicluster-subject` + +Tests that Notes can reference subjects (ConfigMaps) in project control planes. + +Validates: +- Note created in project control plane can reference ConfigMap in same control plane +- Owner reference is correctly set on the Note +- Note is garbage collected when ConfigMap is deleted + + +## Steps + +| # | Name | Bindings | Try | Catch | Finally | Cleanup | +|:-:|---|:-:|:-:|:-:|:-:|:-:| +| 1 | [setup-organization](#step-setup-organization) | 0 | 2 | 0 | 0 | 0 | +| 2 | [create-project](#step-create-project) | 0 | 2 | 0 | 0 | 0 | +| 3 | [create-configmap-in-project](#step-create-configmap-in-project) | 0 | 2 | 0 | 0 | 0 | +| 4 | [create-note-referencing-configmap](#step-create-note-referencing-configmap) | 0 | 2 | 0 | 0 | 0 | +| 5 | [delete-configmap-verify-note-deletion](#step-delete-configmap-verify-note-deletion) | 0 | 2 | 0 | 0 | 0 | + +### Step: `setup-organization` + +Create test organization + +#### Try + +| # | Operation | Bindings | Outputs | Description | +|:-:|---|:-:|:-:|---| +| 1 | `apply` | 0 | 0 | *No description* | +| 2 | `wait` | 0 | 0 | *No description* | + +### Step: `create-project` + +Create project in org control plane + +#### Try + +| # | Operation | Bindings | Outputs | Description | +|:-:|---|:-:|:-:|---| +| 1 | `apply` | 0 | 0 | *No description* | +| 2 | `wait` | 0 | 0 | *No description* | + +### Step: `create-configmap-in-project` + +Create ConfigMap resource in project control plane + +#### Try + +| # | Operation | Bindings | Outputs | Description | +|:-:|---|:-:|:-:|---| +| 1 | `apply` | 0 | 0 | *No description* | +| 2 | `assert` | 0 | 0 | *No description* | + +### Step: `create-note-referencing-configmap` + +Create Note referencing the ConfigMap in project control plane + +#### Try + +| # | Operation | Bindings | Outputs | Description | +|:-:|---|:-:|:-:|---| +| 1 | `apply` | 0 | 0 | *No description* | +| 2 | `assert` | 0 | 0 | *No description* | + +### Step: `delete-configmap-verify-note-deletion` + +Delete ConfigMap and verify Note is garbage collected + +#### Try + +| # | Operation | Bindings | Outputs | Description | +|:-:|---|:-:|:-:|---| +| 1 | `delete` | 0 | 0 | *No description* | +| 2 | `wait` | 0 | 0 | *No description* | + +--- + diff --git a/test/quota/structured-type-enforcement/README.md b/test/quota/structured-type-enforcement/README.md new file mode 100644 index 00000000..3a39cb6e --- /dev/null +++ b/test/quota/structured-type-enforcement/README.md @@ -0,0 +1,205 @@ +# Test: `structured-type-enforcement` + +Tests quota enforcement for EndpointSlice (a structured/native k8s type) +in a project control plane. + +EndpointSlice is a native Kubernetes type that arrives at the admission +plugin as a Go struct (*discoveryv1.EndpointSlice), not as +*unstructured.Unstructured. The admission plugin must convert it to +unstructured with correct JSON field names (metadata, not objectMeta) +for CEL template expressions like trigger.metadata.name to work. + +This test uses a deterministic claim name template +"endpointslice-{{ trigger.metadata.name }}" to verify the CEL +evaluation works correctly for structured types. + + +## Steps + +| # | Name | Bindings | Try | Catch | Finally | Cleanup | +|:-:|---|:-:|:-:|:-:|:-:|:-:| +| 1 | [setup-resource-registration](#step-setup-resource-registration) | 0 | 2 | 0 | 0 | 0 | +| 2 | [setup-grant-creation-policy](#step-setup-grant-creation-policy) | 0 | 2 | 0 | 0 | 0 | +| 3 | [setup-test-organization](#step-setup-test-organization) | 0 | 2 | 0 | 0 | 0 | +| 4 | [create-project-in-org](#step-create-project-in-org) | 0 | 2 | 0 | 0 | 0 | +| 5 | [verify-grant-for-project](#step-verify-grant-for-project) | 0 | 1 | 0 | 0 | 0 | +| 6 | [verify-bucket-pre-created](#step-verify-bucket-pre-created) | 0 | 1 | 0 | 0 | 0 | +| 7 | [setup-claim-creation-policy](#step-setup-claim-creation-policy) | 0 | 2 | 0 | 0 | 0 | +| 8 | [create-endpointslice-1](#step-create-endpointslice-1) | 0 | 2 | 2 | 0 | 0 | +| 9 | [verify-claim-for-endpointslice-1](#step-verify-claim-for-endpointslice-1) | 0 | 1 | 0 | 0 | 0 | +| 10 | [verify-bucket-usage-1-of-5](#step-verify-bucket-usage-1-of-5) | 0 | 1 | 0 | 0 | 0 | +| 11 | [create-endpointslice-2](#step-create-endpointslice-2) | 0 | 1 | 0 | 0 | 0 | +| 12 | [verify-claim-for-endpointslice-2](#step-verify-claim-for-endpointslice-2) | 0 | 1 | 0 | 0 | 0 | +| 13 | [verify-bucket-usage-2-of-5](#step-verify-bucket-usage-2-of-5) | 0 | 1 | 0 | 0 | 0 | +| 14 | [delete-endpointslice-1](#step-delete-endpointslice-1) | 0 | 1 | 0 | 0 | 0 | +| 15 | [verify-bucket-after-deletion](#step-verify-bucket-after-deletion) | 0 | 1 | 0 | 0 | 0 | + +### Step: `setup-resource-registration` + +Register EndpointSlice resource type for quota tracking + +#### Try + +| # | Operation | Bindings | Outputs | Description | +|:-:|---|:-:|:-:|---| +| 1 | `apply` | 0 | 0 | *No description* | +| 2 | `wait` | 0 | 0 | *No description* | + +### Step: `setup-grant-creation-policy` + +Create GrantCreationPolicy to grant EndpointSlice quota to projects + +#### Try + +| # | Operation | Bindings | Outputs | Description | +|:-:|---|:-:|:-:|---| +| 1 | `apply` | 0 | 0 | *No description* | +| 2 | `wait` | 0 | 0 | *No description* | + +### Step: `setup-test-organization` + +Create test organization + +#### Try + +| # | Operation | Bindings | Outputs | Description | +|:-:|---|:-:|:-:|---| +| 1 | `apply` | 0 | 0 | *No description* | +| 2 | `wait` | 0 | 0 | *No description* | + +### Step: `create-project-in-org` + +Create project in org control plane + +#### Try + +| # | Operation | Bindings | Outputs | Description | +|:-:|---|:-:|:-:|---| +| 1 | `apply` | 0 | 0 | *No description* | +| 2 | `wait` | 0 | 0 | *No description* | + +### Step: `verify-grant-for-project` + +Confirm grant is created in project control plane + +#### Try + +| # | Operation | Bindings | Outputs | Description | +|:-:|---|:-:|:-:|---| +| 1 | `wait` | 0 | 0 | *No description* | + +### Step: `verify-bucket-pre-created` + +Verify AllowanceBucket shows 5 available EndpointSlices + +#### Try + +| # | Operation | Bindings | Outputs | Description | +|:-:|---|:-:|:-:|---| +| 1 | `assert` | 0 | 0 | *No description* | + +### Step: `setup-claim-creation-policy` + +Register ClaimCreationPolicy for EndpointSlices with deterministic name + +#### Try + +| # | Operation | Bindings | Outputs | Description | +|:-:|---|:-:|:-:|---| +| 1 | `apply` | 0 | 0 | *No description* | +| 2 | `wait` | 0 | 0 | *No description* | + +### Step: `create-endpointslice-1` + +Create EndpointSlice in project control plane. +This is the critical test: EndpointSlice is a native k8s type that +arrives as a structured Go type in admission. The CEL template +trigger.metadata.name must resolve correctly. + + +#### Try + +| # | Operation | Bindings | Outputs | Description | +|:-:|---|:-:|:-:|---| +| 1 | `sleep` | 0 | 0 | *No description* | +| 2 | `apply` | 0 | 0 | *No description* | + +#### Catch + +| # | Operation | Bindings | Outputs | Description | +|:-:|---|:-:|:-:|---| +| 1 | `script` | 0 | 0 | *No description* | +| 2 | `script` | 0 | 0 | *No description* | + +### Step: `verify-claim-for-endpointslice-1` + +Confirm ResourceClaim with deterministic name is created and granted + +#### Try + +| # | Operation | Bindings | Outputs | Description | +|:-:|---|:-:|:-:|---| +| 1 | `wait` | 0 | 0 | *No description* | + +### Step: `verify-bucket-usage-1-of-5` + +Verify bucket shows 1 EndpointSlice allocated + +#### Try + +| # | Operation | Bindings | Outputs | Description | +|:-:|---|:-:|:-:|---| +| 1 | `assert` | 0 | 0 | *No description* | + +### Step: `create-endpointslice-2` + +Create second EndpointSlice + +#### Try + +| # | Operation | Bindings | Outputs | Description | +|:-:|---|:-:|:-:|---| +| 1 | `apply` | 0 | 0 | *No description* | + +### Step: `verify-claim-for-endpointslice-2` + +Confirm second claim with deterministic name is created and granted + +#### Try + +| # | Operation | Bindings | Outputs | Description | +|:-:|---|:-:|:-:|---| +| 1 | `wait` | 0 | 0 | *No description* | + +### Step: `verify-bucket-usage-2-of-5` + +Verify bucket shows 2 EndpointSlices allocated + +#### Try + +| # | Operation | Bindings | Outputs | Description | +|:-:|---|:-:|:-:|---| +| 1 | `assert` | 0 | 0 | *No description* | + +### Step: `delete-endpointslice-1` + +Delete first EndpointSlice to free quota + +#### Try + +| # | Operation | Bindings | Outputs | Description | +|:-:|---|:-:|:-:|---| +| 1 | `delete` | 0 | 0 | *No description* | + +### Step: `verify-bucket-after-deletion` + +Verify bucket shows quota freed after deletion + +#### Try + +| # | Operation | Bindings | Outputs | Description | +|:-:|---|:-:|:-:|---| +| 1 | `assert` | 0 | 0 | *No description* | + +--- + From 0a99897d520bc062173fd9e6b4277032360f47ae Mon Sep 17 00:00:00 2001 From: Jose Szychowski Date: Wed, 1 Apr 2026 14:46:28 -0300 Subject: [PATCH 17/20] docs: add MachineAccountKey resource documentation and update table formatting --- docs/api/identity.md | 45 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/docs/api/identity.md b/docs/api/identity.md index edc68e08..d2cc07db 100644 --- a/docs/api/identity.md +++ b/docs/api/identity.md @@ -12,6 +12,7 @@ Package v1alpha1 contains API types for identity-related resources. - [UserIdentity](#useridentity) - [Session](#session) +- [MachineAccountKey](#machineaccountkey) --- @@ -22,11 +23,13 @@ UserIdentity represents a user's linked identity within an external identity pro This resource describes the connection between a Milo user and their account in an external authentication provider (e.g., GitHub, Google, Microsoft). It is NOT the identity provider itself, but rather the user's specific identity within that provider. **Use cases:** + - Display all authentication methods linked to a user account in the UI - Show which external accounts a user has connected - Provide visibility into federated identity mappings **Important notes:** + - This is a read-only resource for display purposes only - Identity management (linking/unlinking providers) is handled by the external authentication provider (e.g., Zitadel), not through this API - No sensitive credentials or tokens are exposed through this resource @@ -34,7 +37,7 @@ This resource describes the connection between a Milo user and their account in #### UserIdentityStatus | Field | Type | Description | -|-------|------|-------------| +| :--- | :--- | :--- | | `userUID` | string | The unique identifier of the Milo user who owns this identity. | | `providerID` | string | The unique identifier of the external identity provider instance. This is typically an internal ID from the authentication system. | | `providerName` | string | The human-readable name of the identity provider. Examples: "GitHub", "Google", "Microsoft", "GitLab" | @@ -49,11 +52,13 @@ Session represents an active user session in the system. This resource provides information about user authentication sessions, including the provider used for authentication and session metadata. **Use cases:** + - Display active sessions for a user - Monitor session activity - Provide session management capabilities in the UI **Important notes:** + - This is a read-only resource - Session lifecycle is managed by the authentication provider - No sensitive session tokens are exposed @@ -61,10 +66,46 @@ This resource provides information about user authentication sessions, including #### SessionStatus | Field | Type | Description | -|-------|------|-------------| +| :--- | :--- | :--- | | `userUID` | string | The unique identifier of the user who owns this session. | | `provider` | string | The authentication provider used for this session. | | `ip` | string | The IP address from which the session was created (optional). | | `fingerprintID` | string | A fingerprint identifier for the session (optional). | | `createdAt` | metav1.Time | The timestamp when the session was created. | | `expiresAt` | *metav1.Time | The timestamp when the session expires (optional). | + +--- + +### MachineAccountKey + +MachineAccountKey represents a credential for a MachineAccount. + +This resource allows users to manage API keys for machine-to-machine authentication. When a MachineAccountKey is created, the system generates a private key that is returned in the status only once. + +**Use cases:** + +- Authenticating external services and automation scripts +- Managing key rotation and expiration +- Auditing machine account activity + +**Important notes:** + +- The `privateKey` is ONLY available in the creation response and is NEVER persisted in the Milo API server. +- Keys can have an optional expiration date. +- Each key is associated with a specific `MachineAccount` identified by its email. + +#### MachineAccountKeySpec + +| Field | Type | Description | +| :--- | :--- | :--- | +| `machineAccountUserName` | string | The email address of the MachineAccount that owns this key. | +| `expirationDate` | metav1.Time | Optional date and time when the key will expire. | +| `publicKey` | string | Optional public key to be registered. If not provided, one will be auto-generated. | + +#### MachineAccountKeyStatus + +| Field | Type | Description | +| :--- | :--- | :--- | +| `authProviderKeyID` | string | Unique identifier for the key in the authentication provider (e.g. Zitadel ID). | +| `privateKey` | string | PEM-encoded RSA private key. Only present in the response of a creation event. | +| `conditions` | []metav1.Condition | Standard Kubernetes conditions for resource status. | From 7bf57744162cca4f9a5f723d81782a3ef8ed4268 Mon Sep 17 00:00:00 2001 From: Jose Szychowski Date: Wed, 1 Apr 2026 15:25:16 -0300 Subject: [PATCH 18/20] feat: disable MachineAccountKeys feature gate by default --- pkg/features/features.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/features/features.go b/pkg/features/features.go index af694a3b..089a058a 100644 --- a/pkg/features/features.go +++ b/pkg/features/features.go @@ -68,7 +68,7 @@ var defaultFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ PreRelease: featuregate.Alpha, }, MachineAccountKeys: { - Default: true, + Default: false, PreRelease: featuregate.Alpha, }, Sessions: { From 6f02eacfeb66a0c209b552d3a8d91f419c77cce9 Mon Sep 17 00:00:00 2001 From: Jose Szychowski Date: Wed, 1 Apr 2026 16:28:13 -0300 Subject: [PATCH 19/20] feat: configure audit policy to redact MachineAccountKey private keys from audit logs --- .../apiserver-audit-logging/audit-policy-configmap.yaml | 8 ++++++++ pkg/apis/identity/v1alpha1/machineaccountkey_types.go | 5 +++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/config/components/apiserver-audit-logging/audit-policy-configmap.yaml b/config/components/apiserver-audit-logging/audit-policy-configmap.yaml index c0f2998e..7689fb71 100644 --- a/config/components/apiserver-audit-logging/audit-policy-configmap.yaml +++ b/config/components/apiserver-audit-logging/audit-policy-configmap.yaml @@ -142,6 +142,14 @@ data: - group: "" # core API group resources: ["secrets", "configmaps"] + # Log MachineAccountKey at Metadata level to redact private key from audit logs + # The privateKey is only returned in the response body on creation, so we omit + # the response to prevent credential leakage in audit logs + - level: Metadata + resources: + - group: "identity.miloapis.com" + resources: ["machineaccountkeys"] + # Log Milo API resources at RequestResponse level to capture full context - level: RequestResponse resources: diff --git a/pkg/apis/identity/v1alpha1/machineaccountkey_types.go b/pkg/apis/identity/v1alpha1/machineaccountkey_types.go index 46fd84d4..dc272116 100644 --- a/pkg/apis/identity/v1alpha1/machineaccountkey_types.go +++ b/pkg/apis/identity/v1alpha1/machineaccountkey_types.go @@ -53,8 +53,9 @@ type MachineAccountKeyStatus struct { // persisted to etcd. Any value present on a GET or LIST response indicates a // bug in the server implementation. // - // Note: private key material will appear in API server audit logs for creation - // events. This matches the behavior of similar systems (GCP service account keys). + // Note: The private key is NOT logged in API server audit logs. The audit policy + // is configured to log MachineAccountKey resources at the Metadata level only, + // which redacts the response body containing the private key. // // +kubebuilder:validation:Optional PrivateKey string `json:"privateKey,omitempty"` From 1f723545306653dbe60856e367f1a0f98677a1c6 Mon Sep 17 00:00:00 2001 From: Jose Szychowski Date: Thu, 2 Apr 2026 10:53:44 -0300 Subject: [PATCH 20/20] feat: change MachineAccount CRD scope from Namespaced to Cluster --- config/crd/bases/iam/iam.miloapis.com_machineaccounts.yaml | 2 +- pkg/apis/iam/v1alpha1/machineaccount_types.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/config/crd/bases/iam/iam.miloapis.com_machineaccounts.yaml b/config/crd/bases/iam/iam.miloapis.com_machineaccounts.yaml index 1e56cf9d..e3845a59 100644 --- a/config/crd/bases/iam/iam.miloapis.com_machineaccounts.yaml +++ b/config/crd/bases/iam/iam.miloapis.com_machineaccounts.yaml @@ -12,7 +12,7 @@ spec: listKind: MachineAccountList plural: machineaccounts singular: machineaccount - scope: Namespaced + scope: Cluster versions: - additionalPrinterColumns: - jsonPath: .status.email diff --git a/pkg/apis/iam/v1alpha1/machineaccount_types.go b/pkg/apis/iam/v1alpha1/machineaccount_types.go index 87cec01b..42b16f77 100644 --- a/pkg/apis/iam/v1alpha1/machineaccount_types.go +++ b/pkg/apis/iam/v1alpha1/machineaccount_types.go @@ -15,7 +15,7 @@ import ( // +kubebuilder:printcolumn:name="Access Token Type",type="string",JSONPath=".spec.accessTokenType" // +kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type=='Ready')].status" // +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp" -// +kubebuilder:resource:scope=Namespaced +// +kubebuilder:resource:scope=Cluster type MachineAccount struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"`