-
Notifications
You must be signed in to change notification settings - Fork 1k
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.
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
feat: Add TLS support to the Operator #4796
Conversation
6ffa5ec
to
84909fc
Compare
b2f5292
to
b72cdac
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
type OfflineTlsConfigs struct { | ||
TlsConfigs `json:",inline"` | ||
// verify the client TLS certificate. | ||
VerifyClient *bool `json:"verifyClient,omitempty"` |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lokeshrangineni thoughts?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
i think this needs the |
Signed-off-by: Tommy Hughes <tohughes@redhat.com>
Signed-off-by: Tommy Hughes <tohughes@redhat.com>
e344b31
to
146c6fb
Compare
Signed-off-by: Tommy Hughes <tohughes@redhat.com>
759ed2f
to
3c70419
Compare
Signed-off-by: Tommy Hughes <tohughes@redhat.com>
3c70419
to
d4c3229
Compare
* add tls support to the operator Signed-off-by: Tommy Hughes <tohughes@redhat.com> * operator tls review fix: if statement Signed-off-by: Tommy Hughes <tohughes@redhat.com> * rebase fixes Signed-off-by: Tommy Hughes <tohughes@redhat.com> * authz rbac fixes Signed-off-by: Tommy Hughes <tohughes@redhat.com> --------- Signed-off-by: Tommy Hughes <tohughes@redhat.com> Signed-off-by: Theodor Mihalache <tmihalac@redhat.com>
* add tls support to the operator Signed-off-by: Tommy Hughes <tohughes@redhat.com> * operator tls review fix: if statement Signed-off-by: Tommy Hughes <tohughes@redhat.com> * rebase fixes Signed-off-by: Tommy Hughes <tohughes@redhat.com> * authz rbac fixes Signed-off-by: Tommy Hughes <tohughes@redhat.com> --------- Signed-off-by: Tommy Hughes <tohughes@redhat.com> Signed-off-by: Theodor Mihalache <tmihalac@redhat.com>
* add tls support to the operator Signed-off-by: Tommy Hughes <tohughes@redhat.com> * operator tls review fix: if statement Signed-off-by: Tommy Hughes <tohughes@redhat.com> * rebase fixes Signed-off-by: Tommy Hughes <tohughes@redhat.com> * authz rbac fixes Signed-off-by: Tommy Hughes <tohughes@redhat.com> --------- Signed-off-by: Tommy Hughes <tohughes@redhat.com>
# [0.42.0](v0.41.0...v0.42.0) (2024-12-05) ### Bug Fixes * Add adapters for sqlite datetime conversion ([#4797](#4797)) ([e198b17](e198b17)) * Added grpcio extras to default feature-server image ([#4737](#4737)) ([e9cd373](e9cd373)) * Changing node version in release ([7089918](7089918)) * Feast create empty online table when FeatureView attribute online=False ([#4666](#4666)) ([237c453](237c453)) * Fix db store types in Operator CRD ([#4798](#4798)) ([f09339e](f09339e)) * Fix the config issue for postgres ([#4776](#4776)) ([a36f7e5](a36f7e5)) * Fixed example materialize-incremental and improved explanation ([#4734](#4734)) ([ca8a7ab](ca8a7ab)) * Fixed SparkSource docstrings so it wouldn't used inhereted class docstrings ([#4722](#4722)) ([32e6aa1](32e6aa1)) * Fixing PGVector integration tests ([#4778](#4778)) ([88a0320](88a0320)) * Incorrect type passed to assert_permissions in materialize endpoints ([#4727](#4727)) ([b72c2da](b72c2da)) * Issue of DataSource subclasses using parent abstract class docstrings ([#4730](#4730)) ([b24acd5](b24acd5)) * Operator envVar positioning & tls.SecretRef.Name ([#4806](#4806)) ([1115d96](1115d96)) * Populates project created_time correctly according to created ti… ([#4686](#4686)) ([a61b93c](a61b93c)) * Reduce feast-server container image size & fix dev image build ([#4781](#4781)) ([ccc9aea](ccc9aea)) * Removed version func from feature_store.py ([#4748](#4748)) ([f902bb9](f902bb9)) * Support registry instantiation for read-only users ([#4719](#4719)) ([ca3d3c8](ca3d3c8)) * Syntax Error in BigQuery While Retrieving Columns that Start wit… ([#4713](#4713)) ([60fbc62](60fbc62)) * Update release version in a pertinent Operator file ([#4708](#4708)) ([764a8a6](764a8a6)) ### Features * Add api contract to fastapi docs ([#4721](#4721)) ([1a165c7](1a165c7)) * Add Couchbase as an online store ([#4637](#4637)) ([824859b](824859b)) * Add Operator support for spec.feastProject & status.applied fields ([#4656](#4656)) ([430ac53](430ac53)) * Add services functionality to Operator ([#4723](#4723)) ([d1d80c0](d1d80c0)) * Add TLS support to the Operator ([#4796](#4796)) ([a617a6c](a617a6c)) * Added feast Go operator db stores support ([#4771](#4771)) ([3302363](3302363)) * Added support for setting env vars in feast services in feast controller ([#4739](#4739)) ([84b24b5](84b24b5)) * Adding docs outlining native Python transformations on singletons ([#4741](#4741)) ([0150278](0150278)) * Adding first feast operator e2e test. ([#4791](#4791)) ([8339f8d](8339f8d)) * Adding github action to run the operator end-to-end tests. ([#4762](#4762)) ([d8ccb00](d8ccb00)) * Adding ssl support for registry server. ([#4718](#4718)) ([ccf7a55](ccf7a55)) * Adding SSL support for the React UI server and feast UI command. ([#4736](#4736)) ([4a89252](4a89252)) * Adding support for native Python transformations on a single dictionary ([#4724](#4724)) ([9bbc1c6](9bbc1c6)) * Adding TLS support for offline server. ([#4744](#4744)) ([5d8d03f](5d8d03f)) * Building the feast image ([#4775](#4775)) ([6635dde](6635dde)) * File persistence definition and implementation ([#4742](#4742)) ([3bad4a1](3bad4a1)) * Object store persistence in operator ([#4758](#4758)) ([0ae86da](0ae86da)) * OIDC authorization in Feast Operator ([#4801](#4801)) ([eb111d6](eb111d6)) * Operator will create k8s serviceaccount for each feast service ([#4767](#4767)) ([cde5760](cde5760)) * Printing more verbose logs when we start the offline server ([#4660](#4660)) ([9d8d3d8](9d8d3d8)) * PVC configuration and impl ([#4750](#4750)) ([785a190](785a190)) * Qdrant vectorstore support ([#4689](#4689)) ([86573d2](86573d2)) * RBAC Authorization in Feast Operator ([#4786](#4786)) ([0ef5acc](0ef5acc)) * Support for nested timestamp fields in Spark Offline store ([#4740](#4740)) ([d4d94f8](d4d94f8)) * Update the go feature server from Expedia code repo. ([#4665](#4665)) ([6406625](6406625)) * Updated feast Go operator db stores ([#4809](#4809)) ([2c5a6b5](2c5a6b5)) * Updated sample secret following review ([#4811](#4811)) ([dc9f825](dc9f825))
What this PR does / why we need it:
With this PR, an Operator user will be able to configure feast services with TLS. If the operator detects it's running in an OpenShift cluster, TLS is enabled by default through the use of the service serving certificates feature.
/health
endpointgrpcurl
initContainer to the online & offlineStore deployments, which will check the registry'sgrpc.health.v1.Health/Check
endpoint before attempting to start those feast services.Which issue(s) this PR fixes:
Fixes #4770
Misc
Example of an OpenShift deployment and TLS being automatically configured. In a k8s cluster, TLS would need to be manually configured in the
FeatureStore.spec
. Any required certs, keys, and k8s secrets/configmaps would also have to be created by the user -Example of a remote registry reference -
Which results in the following client configMap & feature_store.yaml -