From aade119b2e4e3ee94d7ad3723b17fca83d813fbf Mon Sep 17 00:00:00 2001 From: Linda Oraegbunam <108290852+obielin@users.noreply.github.com> Date: Wed, 19 Aug 2026 07:45:49 +0100 Subject: [PATCH 1/2] docs: Add end-to-end registry deletion lifecycle example and unit test (#5360) (#6504) * docs: add end-to-end registry deletion lifecycle snippet (#5360) The existing registry deletion docs cover the CLI and individual Python SDK delete methods, but lack a single copy-pasteable example showing the full create -> verify -> delete -> confirm flow. - Add an 'End-to-end example' snippet to registry.md that lists a feature view, deletes it with delete_feature_view(), and lists again to confirm. - Add a unit test for FeatureView deletion via apply(objects_to_delete=..., partial=False) to guard the programmatic deletion path. Signed-off-by: Linda Oraegbunam * test: cover delete_feature_view, the API the docs example uses Addresses review feedback on #6504: the new docs snippet demonstrates store.delete_feature_view(name), but the only test added went through apply(objects_to_delete=..., partial=False), so the documented API was still untested. - Add test_delete_feature_view, mirroring the registry.md snippet step for step: list, delete by name, list again, then assert get_feature_view raises FeatureViewNotFoundException. - Add test_delete_feature_view_raises_when_missing, covering the FeatureViewNotFoundException that delete_feature_view's own docstring promises for an unregistered name. - Keep the apply(objects_to_delete=...) test for the `feast apply` path documented in the hint block, and note in its docstring that it is deliberately distinct from delete_feature_view. - Lift the shared source frame and registration into two helpers so the two lifecycle tests do not duplicate ~30 lines of setup. Signed-off-by: Linda Oraegbunam --------- Signed-off-by: Linda Oraegbunam --- docs/getting-started/components/registry.md | 24 ++++ .../test_local_feature_store.py | 117 +++++++++++++++++- 2 files changed, 140 insertions(+), 1 deletion(-) diff --git a/docs/getting-started/components/registry.md b/docs/getting-started/components/registry.md index 9723e6cfe63..34beae25084 100644 --- a/docs/getting-started/components/registry.md +++ b/docs/getting-started/components/registry.md @@ -69,6 +69,30 @@ store._registry.delete_validation_reference("my_validation_reference", project=s When using `feast apply` via the CLI, you can also use the `objects_to_delete` parameter with `partial=False` to delete objects as part of the apply operation. However, this is less common and typically used in automated deployment scenarios. {% endhint %} +### End-to-end example + +The following snippet shows the full lifecycle of deleting a feature view from the registry: + +```python +from feast import FeatureStore + +store = FeatureStore(repo_path=".") + +# 1. Verify the object exists before deletion +print(store.list_batch_feature_views()) # shows my_feature_view + +# 2. Delete the feature view +store.delete_feature_view("my_feature_view") + +# 3. Confirm it's gone +print(store.list_batch_feature_views()) # my_feature_view no longer listed + +# Trying to fetch it now raises FeatureViewNotFoundException +# store.get_feature_view("my_feature_view") +``` + +The same pattern works for other registry objects: list/verify the object, call the corresponding `delete_*` method, then list again to confirm the deletion. + ## Accessing the registry from clients Users can specify the registry through a `feature_store.yaml` config file, or programmatically. We often see teams diff --git a/sdk/python/tests/unit/local_feast_tests/test_local_feature_store.py b/sdk/python/tests/unit/local_feast_tests/test_local_feature_store.py index ec2513f0726..63f51bcef6e 100644 --- a/sdk/python/tests/unit/local_feast_tests/test_local_feature_store.py +++ b/sdk/python/tests/unit/local_feast_tests/test_local_feature_store.py @@ -11,7 +11,7 @@ from feast.data_format import AvroFormat, ParquetFormat from feast.data_source import KafkaSource from feast.entity import Entity -from feast.errors import ConflictingFeatureViewNames +from feast.errors import ConflictingFeatureViewNames, FeatureViewNotFoundException from feast.feast_object import ALL_RESOURCE_TYPES from feast.feature_store import FeatureStore from feast.feature_view import DUMMY_ENTITY_ID, DUMMY_ENTITY_NAME, FeatureView @@ -443,6 +443,121 @@ def test_apply_permissions(test_feature_store): test_feature_store.teardown() +def _apply_feature_view_to_delete(test_feature_store, file_source): + """Register an entity and a feature view, and return the feature view.""" + entity = Entity( + name="driver_entity", join_keys=["test_key"], value_type=ValueType.INT64 + ) + driver_fv = FeatureView( + name="driver_fv_to_delete", + entities=[entity], + schema=[Field(name="test_key", dtype=Int64)], + source=file_source, + ) + test_feature_store.apply([entity, driver_fv]) + + fvs = test_feature_store.list_batch_feature_views() + assert len(fvs) == 1 + assert fvs[0].name == driver_fv.name + + return driver_fv + + +def _deletion_source_dataframe(): + """Build the small source frame both deletion tests register against.""" + now = pd.Timestamp.utcnow().round("ms") + return pd.DataFrame( + { + "test_key": [1, 2, 1, 3, 3], + "feature_value": [0.1, 0.2, 0.3, 4.0, 5.0], + "ts_1": [ + now, + now - pd.Timedelta(hours=4), + now - pd.Timedelta(hours=3), + now - pd.Timedelta(hours=2), + now - pd.Timedelta(hours=1), + ], + } + ) + + +@pytest.mark.parametrize( + "test_feature_store", + [lazy_fixture("feature_store_with_local_registry")], +) +def test_delete_feature_view(test_feature_store): + """Test the delete_feature_view lifecycle documented in registry.md. + + Mirrors the end-to-end snippet in docs/getting-started/components/registry.md: + list the object, delete it by name, list again to confirm it is gone, and + check that fetching it afterwards raises FeatureViewNotFoundException. + """ + assert isinstance(test_feature_store, FeatureStore) + + with prep_file_source( + df=_deletion_source_dataframe(), timestamp_field="ts_1" + ) as file_source: + driver_fv = _apply_feature_view_to_delete(test_feature_store, file_source) + + # Delete the feature view by name + test_feature_store.delete_feature_view(driver_fv.name) + + # Verify feature view is deleted + assert len(test_feature_store.list_batch_feature_views()) == 0 + + # Verify get_feature_view raises FeatureViewNotFoundException + with pytest.raises(FeatureViewNotFoundException): + test_feature_store.get_feature_view(driver_fv.name) + + test_feature_store.teardown() + + +@pytest.mark.parametrize( + "test_feature_store", + [lazy_fixture("feature_store_with_local_registry")], +) +def test_delete_feature_view_raises_when_missing(test_feature_store): + """Deleting a feature view that was never registered raises, as documented.""" + assert isinstance(test_feature_store, FeatureStore) + + with pytest.raises(FeatureViewNotFoundException): + test_feature_store.delete_feature_view("feature_view_that_does_not_exist") + + test_feature_store.teardown() + + +@pytest.mark.parametrize( + "test_feature_store", + [lazy_fixture("feature_store_with_local_registry")], +) +def test_apply_delete_feature_view(test_feature_store): + """Test that a feature view can be deleted using objects_to_delete with partial=False. + + This is the `feast apply` path called out in the hint block in registry.md, + and is distinct from the delete_feature_view path covered above. + """ + assert isinstance(test_feature_store, FeatureStore) + + with prep_file_source( + df=_deletion_source_dataframe(), timestamp_field="ts_1" + ) as file_source: + driver_fv = _apply_feature_view_to_delete(test_feature_store, file_source) + + # Delete the feature view using objects_to_delete + test_feature_store.apply( + objects=[], objects_to_delete=[driver_fv], partial=False + ) + + # Verify feature view is deleted + assert len(test_feature_store.list_batch_feature_views()) == 0 + + # Verify get_feature_view raises FeatureViewNotFoundException + with pytest.raises(FeatureViewNotFoundException): + test_feature_store.get_feature_view(driver_fv.name) + + test_feature_store.teardown() + + @pytest.mark.parametrize( "test_feature_store", [lazy_fixture("feature_store_with_local_registry")], From e79bd331694ffc7dd6023465b17348470afbe4e6 Mon Sep 17 00:00:00 2001 From: Nikhil Kathole Date: Wed, 19 Aug 2026 13:50:30 +0530 Subject: [PATCH 2/2] ci: Add bundle-sync verification to operator PR workflow (#6751) * ci: Add bundle-sync verification to operator PR workflow Signed-off-by: ntkathole * fix: Regenerate operator bundle to sync RBAC permissions Run `make bundle` to pick up refined clusterrole/clusterrolebinding RBAC rules and drop stale subjectaccessreviews permission. Signed-off-by: ntkathole * fix: Retry on AlreadyExists for cluster-scoped RBAC resources retry.RetryOnConflict only handles Conflict (resource version mismatch). When concurrent reconcile loops both GET a NotFound resource and race to Create it, the loser gets AlreadyExists which was not retried. Switch to retry.OnError with a predicate covering both IsConflict and IsAlreadyExists so the retry re-GETs the now-existing resource and proceeds with an update. Signed-off-by: ntkathole Co-authored-by: Cursor * fix: Regenerate operator bundle to sync RBAC and CRD changes Run make bundle to pick up the clusterrole get/list RBAC verbs and updated CRD field descriptions. Signed-off-by: ntkathole Co-authored-by: Cursor * fix: Handle AlreadyExists and Conflict as non-errors for cluster RBAC The controller-runtime cached client can return stale NotFound for cluster-scoped resources when the informer cache has not yet synced. This causes CreateOrUpdate to attempt a Create that fails with AlreadyExists. Retrying does not help because the cache remains stale during the short retry window. Treat AlreadyExists and Conflict as non-errors since the resource exists in the desired state. The next reconcile cycle will update its contents once the cache has synced. This follows the standard Kubernetes operator eventual-consistency pattern. Signed-off-by: ntkathole Co-authored-by: Cursor --------- Signed-off-by: ntkathole Co-authored-by: Cursor --- .github/workflows/operator_pr.yml | 13 +++++ .../feast-operator.clusterserviceversion.yaml | 30 ++++++++++- .../manifests/feast.dev_featurestores.yaml | 10 +--- .../internal/controller/authz/authz.go | 50 +++++++++++-------- 4 files changed, 72 insertions(+), 31 deletions(-) diff --git a/.github/workflows/operator_pr.yml b/.github/workflows/operator_pr.yml index ff423e65965..2033fb06baf 100644 --- a/.github/workflows/operator_pr.yml +++ b/.github/workflows/operator_pr.yml @@ -19,3 +19,16 @@ jobs: run: make -C infra/feast-operator test - name: After code formatting, check for uncommitted differences run: git diff --exit-code infra/feast-operator + - name: Regenerate bundle and verify CSV is in sync + run: make -C infra/feast-operator bundle + - name: Check for uncommitted bundle differences + run: | + # createdAt and operator-sdk builder version change every run; + # ignore them so only real RBAC / structural drift fails the check. + if ! git diff --exit-code \ + -I 'createdAt:' \ + -I 'operator-sdk-v' \ + infra/feast-operator/bundle/ infra/feast-operator/bundle.Dockerfile; then + echo "::error::Bundle manifests are out of sync. Run 'make bundle' in infra/feast-operator/ and commit the result." + exit 1 + fi diff --git a/infra/feast-operator/bundle/manifests/feast-operator.clusterserviceversion.yaml b/infra/feast-operator/bundle/manifests/feast-operator.clusterserviceversion.yaml index 5fa81e19a1c..6f5e521e12a 100644 --- a/infra/feast-operator/bundle/manifests/feast-operator.clusterserviceversion.yaml +++ b/infra/feast-operator/bundle/manifests/feast-operator.clusterserviceversion.yaml @@ -147,7 +147,7 @@ metadata: } ] capabilities: Basic Install - createdAt: "2026-07-31T18:28:04Z" + createdAt: "2026-08-18T16:15:46Z" operators.operatorframework.io/builder: operator-sdk-v1.41.0 operators.operatorframework.io/project_layout: go.kubebuilder.io/v4 name: feast-operator.v0.65.0 @@ -342,10 +342,36 @@ spec: - rbac.authorization.k8s.io resources: - clusterrolebindings + verbs: + - create + - delete + - get + - list + - update + - apiGroups: + - rbac.authorization.k8s.io + resources: + - clusterroles + verbs: + - create + - get + - list + - apiGroups: + - rbac.authorization.k8s.io + resourceNames: + - feast-discover-namespaces + - feast-oidc-token-review + - feast-token-review-cluster-role + resources: - clusterroles + verbs: + - delete + - update + - apiGroups: + - rbac.authorization.k8s.io + resources: - rolebindings - roles - - subjectaccessreviews verbs: - create - delete diff --git a/infra/feast-operator/bundle/manifests/feast.dev_featurestores.yaml b/infra/feast-operator/bundle/manifests/feast.dev_featurestores.yaml index 7be8b085cd8..2005aa78c5f 100644 --- a/infra/feast-operator/bundle/manifests/feast.dev_featurestores.yaml +++ b/infra/feast-operator/bundle/manifests/feast.dev_featurestores.yaml @@ -59,8 +59,6 @@ spec: type: object noAuth: description: NoAuth explicitly disables authentication and authorization. - When set to true, Feast services run without any auth checks. - Use only for development or testing environments. type: boolean oidc: description: |- @@ -6458,8 +6456,7 @@ spec: type: object noAuth: description: NoAuth explicitly disables authentication and - authorization. When set to true, Feast services run without - any auth checks. Use only for development or testing environments. + authorization. type: boolean oidc: description: |- @@ -13065,8 +13062,6 @@ spec: type: object noAuth: description: NoAuth explicitly disables authentication and authorization. - When set to true, Feast services run without any auth checks. - Use only for development or testing environments. type: boolean oidc: description: |- @@ -17581,8 +17576,7 @@ spec: type: object noAuth: description: NoAuth explicitly disables authentication and - authorization. When set to true, Feast services run without - any auth checks. Use only for development or testing environments. + authorization. type: boolean oidc: description: |- diff --git a/infra/feast-operator/internal/controller/authz/authz.go b/infra/feast-operator/internal/controller/authz/authz.go index b37643f0181..7a5354c1335 100644 --- a/infra/feast-operator/internal/controller/authz/authz.go +++ b/infra/feast-operator/internal/controller/authz/authz.go @@ -8,10 +8,10 @@ import ( feastdevv1 "github.com/feast-dev/feast/infra/feast-operator/api/v1" "github.com/feast-dev/feast/infra/feast-operator/internal/controller/services" rbacv1 "k8s.io/api/rbac/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" - "k8s.io/client-go/util/retry" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/log" @@ -146,17 +146,21 @@ func (authz *FeastAuthorization) createFeastRole() error { func (authz *FeastAuthorization) createFeastClusterRole() error { logger := log.FromContext(authz.Handler.Context) - return retry.RetryOnConflict(retry.DefaultRetry, func() error { - clusterRole := authz.initFeastClusterRole() - if op, err := controllerutil.CreateOrUpdate(authz.Handler.Context, authz.Handler.Client, clusterRole, controllerutil.MutateFn(func() error { - return authz.setFeastClusterRole(clusterRole) - })); err != nil { - return err - } else if op == controllerutil.OperationResultCreated || op == controllerutil.OperationResultUpdated { - logger.Info("Successfully reconciled", "ClusterRole", clusterRole.Name, "operation", op) - } + clusterRole := authz.initFeastClusterRole() + op, err := controllerutil.CreateOrUpdate(authz.Handler.Context, authz.Handler.Client, clusterRole, controllerutil.MutateFn(func() error { + return authz.setFeastClusterRole(clusterRole) + })) + if apierrors.IsAlreadyExists(err) || apierrors.IsConflict(err) { + logger.Info("ClusterRole conflict or already exists, will reconcile on next cycle", "ClusterRole", clusterRole.Name, "error", err) return nil - }) + } + if err != nil { + return err + } + if op == controllerutil.OperationResultCreated || op == controllerutil.OperationResultUpdated { + logger.Info("Successfully reconciled", "ClusterRole", clusterRole.Name, "operation", op) + } + return nil } func (authz *FeastAuthorization) initFeastClusterRole() *rbacv1.ClusterRole { @@ -228,17 +232,21 @@ func (authz *FeastAuthorization) setFeastClusterRoleBinding(clusterRoleBinding * // Create ClusterRoleBinding func (authz *FeastAuthorization) createFeastClusterRoleBinding() error { logger := log.FromContext(authz.Handler.Context) - return retry.RetryOnConflict(retry.DefaultRetry, func() error { - clusterRoleBinding := authz.initFeastClusterRoleBinding() - if op, err := controllerutil.CreateOrUpdate(authz.Handler.Context, authz.Handler.Client, clusterRoleBinding, controllerutil.MutateFn(func() error { - return authz.setFeastClusterRoleBinding(clusterRoleBinding) - })); err != nil { - return err - } else if op == controllerutil.OperationResultCreated || op == controllerutil.OperationResultUpdated { - logger.Info("Successfully reconciled", "ClusterRoleBinding", clusterRoleBinding.Name, "operation", op) - } + clusterRoleBinding := authz.initFeastClusterRoleBinding() + op, err := controllerutil.CreateOrUpdate(authz.Handler.Context, authz.Handler.Client, clusterRoleBinding, controllerutil.MutateFn(func() error { + return authz.setFeastClusterRoleBinding(clusterRoleBinding) + })) + if apierrors.IsAlreadyExists(err) || apierrors.IsConflict(err) { + logger.Info("ClusterRoleBinding conflict or already exists, will reconcile on next cycle", "ClusterRoleBinding", clusterRoleBinding.Name, "error", err) return nil - }) + } + if err != nil { + return err + } + if op == controllerutil.OperationResultCreated || op == controllerutil.OperationResultUpdated { + logger.Info("Successfully reconciled", "ClusterRoleBinding", clusterRoleBinding.Name, "operation", op) + } + return nil } func (authz *FeastAuthorization) initFeastRole() *rbacv1.Role {