Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add OIDC Federation Settings #479

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+395 −23
Open

Add OIDC Federation Settings #479

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 90 additions & 2 deletions api/bases/keystone.openstack.org_keystoneapis.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ spec:
files. Those get added to the service config dir in /etc/<service>
. TODO: -> implement'
type: object
enableFederation:
default: false
description: Enablement of Federation configuration
type: boolean
enableSecureRBAC:
default: true
description: EnableSecureRBAC - Enable Consistent and Secure RBAC
Expand Down Expand Up @@ -119,6 +123,76 @@ spec:
description: NodeSelector to target subset of worker nodes running
this service
type: object
oidcFederation:
description: KeystoneFederationSpec to provide the configuration values
for OIDC Federation
properties:
keystoneFederationIdentityProviderName:
default: ""
description: KeystoneFederationIdentityProviderName
type: string
oidcCacheType:
default: memcache
description: OIDCCacheType
type: string
oidcClaimDelimiter:
default: ;
description: OIDCClaimDelimiter
type: string
oidcClaimPrefix:
default: OIDC-
description: OIDCClaimPrefix
type: string
oidcClientID:
default: ""
description: OIDCClientID
type: string
oidcIntrospectionEndpoint:
default: ""
description: OIDCIntrospectionEndpoint
type: string
oidcMemCacheServers:
description: OIDCMemCacheServers
type: string
oidcPassClaimsAs:
default: both
description: OIDCPassClaimsAs
type: string
oidcPassUserInfoAs:
default: claims
description: OIDCPassUserInfoAs
type: string
oidcProviderMetadataURL:
default: ""
description: OIDCProviderMetadataURL
type: string
oidcResponseType:
default: id_token
description: OIDCResponseType
type: string
oidcScope:
default: openid email profile
description: OIDCScope
type: string
remoteIDAttribute:
default: HTTP_OIDC_ISS
description: RemoteIDAttribute
type: string
required:
- keystoneFederationIdentityProviderName
- oidcCacheType
- oidcClaimDelimiter
- oidcClaimPrefix
- oidcClientID
- oidcIntrospectionEndpoint
- oidcMemCacheServers
- oidcPassClaimsAs
- oidcPassUserInfoAs
- oidcProviderMetadataURL
- oidcResponseType
- oidcScope
- remoteIDAttribute
type: object
override:
description: Override, provides the ability to override the generated
manifest of several child resources.
Expand Down Expand Up @@ -296,14 +370,27 @@ spec:
passwordSelectors:
default:
admin: AdminPassword
description: PasswordSelectors - Selectors to identify the AdminUser
password from the Secret
keystoneOIDCClientSecret: KeystoneClientSecret
keystoneOIDCCryptoPassphrase: KeystoneCryptoPassphrase
description: PasswordSelectors - Selectors to identify the AdminUser,
KeystoneOIDCClient, and KeystoneOIDCCryptoPassphrase passwords from
the Secret
properties:
admin:
default: AdminPassword
description: Admin - Selector to get the keystone Admin password
from the Secret
type: string
keystoneOIDCClientSecret:
default: KeystoneClientSecret
description: OIDCClientSecret - Selector to get the IdP client
secret from the Secret
type: string
keystoneOIDCCryptoPassphrase:
default: KeystoneCryptoPassphrase
description: OIDCCryptoPassphrase - Selector to get the OIDC crypto
passphrase from the Secret
type: string
type: object
preserveJobs:
default: false
Expand Down Expand Up @@ -426,6 +513,7 @@ spec:
required:
- containerImage
- databaseInstance
- enableFederation
- memcachedInstance
- rabbitMqClusterName
- secret
Expand Down
92 changes: 89 additions & 3 deletions api/v1beta1/keystoneapi_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,8 @@ type KeystoneAPISpecCore struct {
FernetMaxActiveKeys *int32 `json:"fernetMaxActiveKeys"`

// +kubebuilder:validation:Optional
// +kubebuilder:default={admin: AdminPassword}
// PasswordSelectors - Selectors to identify the AdminUser password from the Secret
// +kubebuilder:default={admin: AdminPassword, keystoneOIDCClientSecret: KeystoneClientSecret, keystoneOIDCCryptoPassphrase: KeystoneCryptoPassphrase}
// PasswordSelectors - Selectors to identify the AdminUser, KeystoneOIDCClient, and KeystoneOIDCCryptoPassphrase passwords from the Secret
PasswordSelectors PasswordSelector `json:"passwordSelectors"`

// +kubebuilder:validation:Optional
Expand Down Expand Up @@ -180,6 +180,15 @@ type KeystoneAPISpecCore struct {
// +operator-sdk:csv:customresourcedefinitions:type=spec
// TLS - Parameters related to the TLS
TLS tls.API `json:"tls,omitempty"`

d34dh0r53 marked this conversation as resolved.
Show resolved Hide resolved
// +kubebuilder:validation:Required
// +kubebuilder:default=false
// Enablement of Federation configuration
EnableFederation bool `json:"enableFederation"`
Comment on lines +184 to +187
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can not be a required field, or we need a new API version so that a conversion webhook could introduce it. for now it should be optional with default to false. see recent discussions on slack.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

best to test this pr using a previous version and update to have this change

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't have to be required, I'll change it to optional

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we even need this field? If you change the federationConfig object to a pointer, you can check for null-ness


// +kubebuilder:validation:Optional
// +OIDCFederation - parameters to configure keystone for OIDC federation
OIDCFederation KeystoneFederationSpec `json:"oidcFederation,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might want to do this a ptr so that it won't show up with all the empty values?

apiVersion: keystone.openstack.org/v1beta1
kind: KeystoneAPI
metadata:
  creationTimestamp: "2024-11-18T19:38:57Z"
  finalizers:
  - openstack.org/keystoneapi
  - openstack.org/keystoneservice-nova
  - openstack.org/keystoneservice-glance
  - openstack.org/keystoneservice-ceilometer
  - openstack.org/keystoneservice-swift
  - openstack.org/keystoneendpoint-glance-default-external
  - openstack.org/keystoneendpoint-glance-default-internal
  - openstack.org/keystoneendpoint-swift
  - openstack.org/keystoneservice-barbican
  - openstack.org/keystoneservice-cinderv3
  - openstack.org/keystoneendpoint-barbican-api
  - openstack.org/keystoneendpoint-cinderv3
  - openstack.org/keystoneservice-neutron
  - openstack.org/keystoneservice-placement
  - openstack.org/keystoneendpoint-neutron
  - openstack.org/keystoneendpoint-placement
  - openstack.org/keystoneendpoint-nova
  generation: 2
  name: keystone
  namespace: openstack
  ownerReferences:
  - apiVersion: core.openstack.org/v1beta1
    blockOwnerDeletion: true
    controller: true
    kind: OpenStackControlPlane
    name: openstack-galera
    uid: 54409ae3-a561-43b7-9251-603437046eb3
  resourceVersion: "70386"
  uid: fbc4eab5-72b6-485e-b9b9-6536919d4c4b
spec:
  adminProject: admin
  adminUser: admin
  containerImage: quay.io/podified-antelope-centos9/openstack-keystone@sha256:198eb79012e1b824b4092e4907f0565051f8514a7ca0d4606e9ce7a3d8439895
  databaseAccount: keystone
  databaseInstance: openstack
  enableFederation: false
  enableSecureRBAC: true
  fernetMaxActiveKeys: 5
  fernetRotationDays: 1
  memcachedInstance: memcached
  oidcFederation:
    keystoneFederationIdentityProviderName: ""
    oidcCacheType: ""
    oidcClaimDelimiter: ""
    oidcClaimPrefix: ""
    oidcClientID: ""
    oidcIntrospectionEndpoint: ""
    oidcMemCacheServers: ""
    oidcPassClaimsAs: ""
    oidcPassUserInfoAs: ""
    oidcProviderMetadataURL: ""
    oidcResponseType: ""
    oidcScope: ""
    remoteIDAttribute: ""

[1] https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/openstack-k8s-operators_keystone-operator/479/pull-ci-openstack-k8s-operators-keystone-operator-main-keystone-operator-build-deploy-kuttl/1858575042430898176/artifacts/keystone-operator-build-deploy-kuttl/openstack-k8s-operators-gather/artifacts/must-gather/quay-io-openstack-k8s-operators-openstack-must-gather-sha256-3d1f620ae3617fbd6371f55aa948b34e9daf31a461d3c557cf1a7c2db38d0385/namespaces/openstack/crs/keystoneapis.keystone.openstack.org/keystone.yaml

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds like it's the solution to my problem, I couldn't figure out why the values were empty when I was specifying defaults. Do you know of any examples of how to do this with a pointer?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just change it to

OIDCFederation *KeystoneFederationSpec `json:"oidcFederation,omitempty"`

probably needs some check for it bein nil where you right now just use it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can see how I'm doing it with the PKCS11 settings in barbican in openstack-k8s-operators/barbican-operator#168

}

// APIOverrideSpec to override the generated manifest of several child resources.
Expand All @@ -195,6 +204,83 @@ type PasswordSelector struct {
// +kubebuilder:default="AdminPassword"
// Admin - Selector to get the keystone Admin password from the Secret
Admin string `json:"admin"`

d34dh0r53 marked this conversation as resolved.
Show resolved Hide resolved
// +kubebuilder:validation:Optional
// +kubebuilder:default="KeystoneClientSecret"
// OIDCClientSecret - Selector to get the IdP client secret from the Secret
KeystoneOIDCClientSecret string `json:"keystoneOIDCClientSecret"`

// +kubebuilder:validation:Optional
// +kubebuilder:default="KeystoneCryptoPassphrase"
// OIDCCryptoPassphrase - Selector to get the OIDC crypto passphrase from the Secret
KeystoneOIDCCryptoPassphrase string `json:"keystoneOIDCCryptoPassphrase"`
}

// KeystoneFederationSpec to provide the configuration values for OIDC Federation
type KeystoneFederationSpec struct {
// +kubebuilder:validation:Required
// +kubebuilder:default="OIDC-"
// OIDCClaimPrefix
OIDCClaimPrefix string `json:"oidcClaimPrefix"`

// +kubebuilder:validation:Required
// +kubebuilder:default="id_token"
// OIDCResponseType
OIDCResponseType string `json:"oidcResponseType"`

// +kubebuilder:validation:Required
// +kubebuilder:default="openid email profile"
// OIDCScope
OIDCScope string `json:"oidcScope"`

// +kubebuilder:validation:Required
// +kubebuilder:default=""
// OIDCProviderMetadataURL
OIDCProviderMetadataURL string `json:"oidcProviderMetadataURL"`

// +kubebuilder:validation:Required
// +kubebuilder:default=""
// OIDCIntrospectionEndpoint
OIDCIntrospectionEndpoint string `json:"oidcIntrospectionEndpoint"`

// +kubebuilder:validation:Required
// +kubebuilder:default=""
// OIDCClientID
OIDCClientID string `json:"oidcClientID"`

// +kubebuilder:validation:Required
// +kubebuilder:default=";"
// OIDCClaimDelimiter
OIDCClaimDelimiter string `json:"oidcClaimDelimiter"`

// +kubebuilder:validation:Required
// +kubebuilder:default="claims"
// OIDCPassUserInfoAs
OIDCPassUserInfoAs string `json:"oidcPassUserInfoAs"`

// +kubebuilder:validation:Required
// +kubebuilder:default="both"
// OIDCPassClaimsAs
OIDCPassClaimsAs string `json:"oidcPassClaimsAs"`

// +kubebuilder:validation:Required
// +kubebuilder:default="memcache"
// OIDCCacheType
OIDCCacheType string `json:"oidcCacheType"`

// +kubebuilder:validaton:Required
// OIDCMemCacheServers
OIDCMemCacheServers string `json:"oidcMemCacheServers"`

// +kubebuilder:validation:Required
// +kubebuilder:default="HTTP_OIDC_ISS"
// RemoteIDAttribute
RemoteIDAttribute string `json:"remoteIDAttribute"`

// +kubebuilder:validation:Required
// +kubebuilder:default=""
// KeystoneFederationIdentityProviderName
KeystoneFederationIdentityProviderName string `json:"keystoneFederationIdentityProviderName"`
}

// KeystoneAPIStatus defines the observed state of KeystoneAPI
Expand All @@ -220,7 +306,7 @@ type KeystoneAPIStatus struct {
// TransportURLSecret - Secret containing RabbitMQ transportURL
TransportURLSecret string `json:"transportURLSecret,omitempty"`

//ObservedGeneration - the most recent generation observed for this service. If the observed generation is less than the spec generation, then the controller has not processed the latest changes.
// ObservedGeneration - the most recent generation observed for this service. If the observed generation is less than the spec generation, then the controller has not processed the latest changes.
ObservedGeneration int64 `json:"observedGeneration,omitempty"`
}

Expand Down
16 changes: 16 additions & 0 deletions api/v1beta1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading