Skip to content

feat: Add TLS support to the Operator #4796

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

Merged
dmartinol merged 4 commits into from
Dec 2, 2024
Merged

feat: Add TLS support to the Operator #4796

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
727883c
add tls support to the operator
tchughesiv Nov 19, 2024
274eb6a
operator tls review fix: if statement
tchughesiv Dec 1, 2024
648f519
rebase fixes
tchughesiv Dec 1, 2024
d4c3229
authz rbac fixes
tchughesiv Dec 1, 2024
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
55 changes: 53 additions & 2 deletions infra/feast-operator/api/v1alpha1/featurestore_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const (
OnlineStoreReadyType = "OnlineStore"
RegistryReadyType = "Registry"
ReadyType = "FeatureStore"
AuthorizationReadyType = "AuthorizationReadyType"
AuthorizationReadyType = "Authorization"

// Feast condition reasons:
ReadyReason = "Ready"
Expand Down Expand Up @@ -76,6 +76,14 @@ type FeatureStoreServices struct {
type OfflineStore struct {
ServiceConfigs `json:",inline"`
Persistence *OfflineStorePersistence `json:"persistence,omitempty"`
TLS *OfflineTlsConfigs `json:"tls,omitempty"`
}

// OfflineTlsConfigs configures server TLS for the offline feast service. in an openshift cluster, this is configured by default using service serving certificates.
type OfflineTlsConfigs struct {
TlsConfigs `json:",inline"`
// verify the client TLS certificate.
VerifyClient *bool `json:"verifyClient,omitempty"`
Copy link
Contributor

@dmartinol dmartinol Nov 28, 2024

Choose a reason for hiding this comment

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

why for offline store we need a different TLS config?

Copy link
Contributor Author

@tchughesiv tchughesiv Nov 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

offline offers that additional TLS related flag for some reason, whereas the others don't.

feast/sdk/python/feast/cli.py

Line 1136 in e198b17

"--verify_client",

that said, its not mentioned in the docs, so maybe its unnecessary? i guess i just felt like we should offer the user all the options. it's certainly added a layer of complexity to the code.

Copy link
Contributor Author

@tchughesiv tchughesiv Nov 28, 2024

Choose a reason for hiding this comment

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

@lokeshrangineni thoughts?

Copy link
Contributor

@lokeshrangineni lokeshrangineni Dec 1, 2024

Choose a reason for hiding this comment

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

Regarding verify_client flag - flight server is very strict with the client validation when we pass the TLS self signed certificates which made it difficult for the integration tests so I had to add this flag for the integration tests. By default verify_client flag is enabled and in the production environment it is supposed to be enabled as well. for the integration tests i marked verify_client as false. I will add about this flag to the documentation as well.

Copy link
Contributor

@dmartinol dmartinol Dec 2, 2024

Choose a reason for hiding this comment

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

can we add the verify_client options to all the servers @lokeshrangineni ?
this way we can keep a consistent CR definition and offer the same set of security functions to all of them

}

// OfflineStorePersistence configures the persistence settings for the offline store service
Expand Down Expand Up @@ -119,6 +127,7 @@ var ValidOfflineStoreDBStorePersistenceTypes = []string{
type OnlineStore struct {
ServiceConfigs `json:",inline"`
Persistence *OnlineStorePersistence `json:"persistence,omitempty"`
TLS *TlsConfigs `json:"tls,omitempty"`
}

// OnlineStorePersistence configures the persistence settings for the online store service
Expand Down Expand Up @@ -163,9 +172,11 @@ var ValidOnlineStoreDBStorePersistenceTypes = []string{
type LocalRegistryConfig struct {
ServiceConfigs `json:",inline"`
Persistence *RegistryPersistence `json:"persistence,omitempty"`
TLS *TlsConfigs `json:"tls,omitempty"`
}

// RegistryPersistence configures the persistence settings for the registry service
// +kubebuilder:validation:XValidation:rule="[has(self.file), has(self.store)].exists_one(c, c)",message="One selection required between file or store."
type RegistryPersistence struct {
FilePersistence *RegistryFilePersistence `json:"file,omitempty"`
DBPersistence *RegistryDBStorePersistence `json:"store,omitempty"`
Expand Down Expand Up @@ -238,7 +249,8 @@ type RemoteRegistryConfig struct {
// Host address of the remote registry service - <domain>:<port>, e.g. `registry.<namespace>.svc.cluster.local:80`
Hostname *string `json:"hostname,omitempty"`
// Reference to an existing `FeatureStore` CR in the same k8s cluster.
FeastRef *FeatureStoreRef `json:"feastRef,omitempty"`
FeastRef *FeatureStoreRef `json:"feastRef,omitempty"`
TLS *TlsRemoteRegistryConfigs `json:"tls,omitempty"`
}

// FeatureStoreRef defines which existing FeatureStore's registry should be used
Expand Down Expand Up @@ -284,6 +296,45 @@ type KubernetesAuthz struct {
Roles []string `json:"roles,omitempty"`
}

// TlsConfigs configures server TLS for a feast service. in an openshift cluster, this is configured by default using service serving certificates.
// +kubebuilder:validation:XValidation:rule="(!has(self.disable) || !self.disable) ? has(self.secretRef) : true",message="`secretRef` required if `disable` is false."
type TlsConfigs struct {
// references the local k8s secret where the TLS key and cert reside
SecretRef *corev1.LocalObjectReference `json:"secretRef,omitempty"`
SecretKeyNames SecretKeyNames `json:"secretKeyNames,omitempty"`
// will disable TLS for the feast service. useful in an openshift cluster, for example, where TLS is configured by default
Disable *bool `json:"disable,omitempty"`
}

// `secretRef` required if `disable` is false.
func (tls *TlsConfigs) IsTLS() bool {
if tls != nil {
if tls.Disable != nil && *tls.Disable {
return false
} else if tls.SecretRef == nil {
return false
}
return true
}
return false
}

// TlsRemoteRegistryConfigs configures client TLS for a remote feast registry. in an openshift cluster, this is configured by default when the remote feast registry is using service serving certificates.
type TlsRemoteRegistryConfigs struct {
// references the local k8s configmap where the TLS cert resides
ConfigMapRef corev1.LocalObjectReference `json:"configMapRef"`
// defines the configmap key name for the client TLS cert.
CertName string `json:"certName"`
}

// SecretKeyNames defines the secret key names for the TLS key and cert.
type SecretKeyNames struct {
// defaults to "tls.crt"
TlsCrt string `json:"tlsCrt,omitempty"`
// defaults to "tls.key"
TlsKey string `json:"tlsKey,omitempty"`
}

// FeatureStoreStatus defines the observed state of FeatureStore
type FeatureStoreStatus struct {
// Shows the currently applied feast configuration, including any pertinent defaults
Expand Down
98 changes: 98 additions & 0 deletions infra/feast-operator/api/v1alpha1/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.

3 changes: 3 additions & 0 deletions infra/feast-operator/cmd/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (

feastdevv1alpha1 "github.com/feast-dev/feast/infra/feast-operator/api/v1alpha1"
"github.com/feast-dev/feast/infra/feast-operator/internal/controller"
"github.com/feast-dev/feast/infra/feast-operator/internal/controller/services"
//+kubebuilder:scaffold:imports
)

Expand Down Expand Up @@ -132,6 +133,8 @@ func main() {
os.Exit(1)
}

services.SetIsOpenShift(mgr.GetConfig())

if err = (&controller.FeatureStoreReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Expand Down
Loading
Loading