Skip to content

Commit 462da5b

Browse files
jyejarejyejare
committed
fix: Address Devin AI and others review comments
Signed-off-by: Jitendra Yejare <11752425+jyejare@users.noreply.github.com>
1 parent e69fa4e commit 462da5b

40 files changed

Lines changed: 419 additions & 345 deletions

‎.secrets.baseline‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -957,7 +957,7 @@
957957
"filename": "infra/feast-operator/api/v1/featurestore_types.go",
958958
"hashed_secret": "44e17306b837162269a410204daaa5ecee4ec22c",
959959
"is_verified": false,
960-
"line_number": 889
960+
"line_number": 895
961961
}
962962
],
963963
"infra/feast-operator/api/v1/zz_generated.deepcopy.go": [
@@ -1110,13 +1110,6 @@
11101110
"is_verified": false,
11111111
"line_number": 295
11121112
},
1113-
{
1114-
"type": "Secret Keyword",
1115-
"filename": "infra/feast-operator/internal/controller/featurestore_controller_db_store_test.go",
1116-
"hashed_secret": "bd29b05f76d7125eb94b34447d9cb77cb98cd55f",
1117-
"is_verified": false,
1118-
"line_number": 679
1119-
},
11201113
{
11211114
"type": "Secret Keyword",
11221115
"filename": "infra/feast-operator/internal/controller/featurestore_controller_db_store_test.go",
@@ -1562,5 +1555,5 @@
15621555
}
15631556
]
15641557
},
1565-
"generated_at": "2026-05-22T11:36:48Z"
1558+
"generated_at": "2026-05-29T12:38:16Z"
15661559
}

‎docs/how-to-guides/feature-monitoring.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ Baseline computation is:
5151
To skip automatic baseline computation on `feast apply`, set the DQM config in `feature_store.yaml`:
5252

5353
```yaml
54-
dqm:
54+
DataQualityMonitoring:
5555
auto_baseline: false
5656
```
5757
@@ -62,7 +62,7 @@ apiVersion: feast.dev/v1
6262
kind: FeatureStore
6363
spec:
6464
feastProject: my_project
65-
dqm:
65+
dataQualityMonitoring:
6666
autoBaseline: false
6767
```
6868

‎infra/feast-operator/.golangci.yml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,11 @@ linters:
3838
path: api/*
3939
- linters:
4040
- dupl
41+
- goconst
4142
- lll
4243
path: internal/*
4344
- linters:
45+
- goconst
4446
- lll
4547
path: test/*
4648
- linters:

‎infra/feast-operator/Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ vet: ## Run go vet against code.
116116

117117
.PHONY: test
118118
test: build-installer vet lint envtest ## Run tests.
119-
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test $$(go list ./... | grep -v test/e2e | grep -v test/data-source-types | grep -v test/upgrade | grep -v test/previous-version) -coverprofile cover.out
119+
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" GOTOOLCHAIN=go$(shell go env GOVERSION | sed 's/^go//') go test $$(go list ./... | grep -v test/e2e | grep -v test/data-source-types | grep -v test/upgrade | grep -v test/previous-version) -coverprofile cover.out
120120

121121
# Utilize Kind or modify the e2e tests to load the image locally, enabling compatibility with other vendors.
122122
.PHONY: test-e2e # Run the e2e tests against a Kind k8s instance that is spun up.

‎infra/feast-operator/api/v1/featurestore_types.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ type FeatureStoreSpec struct {
129129
BatchEngine *BatchEngineConfig `json:"batchEngine,omitempty"`
130130
// DataQualityMonitoring configures Data Quality Monitoring behaviour.
131131
// +optional
132-
DataQualityMonitoring *DataQualityMonitoringConfig `json:"dqm,omitempty"`
132+
DataQualityMonitoring *DataQualityMonitoringConfig `json:"dataQualityMonitoring,omitempty"`
133133
// Replicas is the desired number of pod replicas. Used by the scale sub-resource.
134134
// Mutually exclusive with services.scaling.autoscaling.
135135
// +kubebuilder:default=1

‎infra/feast-operator/api/v1/zz_generated.deepcopy.go‎

Lines changed: 14 additions & 14 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -529,8 +529,9 @@ spec:
529529
description: The time zone name for the given schedule, see https://en.wikipedia.org/wiki/List_of_tz_database_time_zones.
530530
type: string
531531
type: object
532-
dqm:
533-
description: DataQualityMonitoring configures Data Quality Monitoring behaviour.
532+
dataQualityMonitoring:
533+
description: DataQualityMonitoring configures Data Quality Monitoring
534+
behaviour.
534535
properties:
535536
autoBaseline:
536537
default: true
@@ -6500,8 +6501,9 @@ spec:
65006501
https://en.wikipedia.org/wiki/List_of_tz_database_time_zones.
65016502
type: string
65026503
type: object
6503-
dqm:
6504-
description: DataQualityMonitoring configures Data Quality Monitoring behaviour.
6504+
dataQualityMonitoring:
6505+
description: DataQualityMonitoring configures Data Quality Monitoring
6506+
behaviour.
65056507
properties:
65066508
autoBaseline:
65076509
default: true

‎infra/feast-operator/dist/install.yaml‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -537,8 +537,9 @@ spec:
537537
description: The time zone name for the given schedule, see https://en.wikipedia.org/wiki/List_of_tz_database_time_zones.
538538
type: string
539539
type: object
540-
dqm:
541-
description: DataQualityMonitoring configures Data Quality Monitoring behaviour.
540+
dataQualityMonitoring:
541+
description: DataQualityMonitoring configures Data Quality Monitoring
542+
behaviour.
542543
properties:
543544
autoBaseline:
544545
default: true
@@ -6508,8 +6509,9 @@ spec:
65086509
https://en.wikipedia.org/wiki/List_of_tz_database_time_zones.
65096510
type: string
65106511
type: object
6511-
dqm:
6512-
description: DataQualityMonitoring configures Data Quality Monitoring behaviour.
6512+
dataQualityMonitoring:
6513+
description: DataQualityMonitoring configures Data Quality Monitoring
6514+
behaviour.
65136515
properties:
65146516
autoBaseline:
65156517
default: true

‎infra/feast-operator/docs/api/markdown/ref.md‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -104,35 +104,35 @@ _Appears in:_
104104
Defaults to "feast apply" & "feast materialize-incremental $(date -u +'%Y-%m-%dT%H:%M:%S')" |
105105

106106

107-
#### DefaultCtrConfigs
107+
#### DataQualityMonitoringConfig
108108

109109

110110

111-
DefaultCtrConfigs k8s container settings that are applied by default
111+
DataQualityMonitoringConfig defines the Data Quality Monitoring configuration.
112112

113113
_Appears in:_
114-
- [ContainerConfigs](#containerconfigs)
115-
- [CronJobContainerConfigs](#cronjobcontainerconfigs)
116-
- [RegistryServerConfigs](#registryserverconfigs)
117-
- [ServerConfigs](#serverconfigs)
114+
- [FeatureStoreSpec](#featurestorespec)
118115

119116
| Field | Description |
120117
| --- | --- |
121-
| `image` _string_ | |
118+
| `autoBaseline` _boolean_ | AutoBaseline controls whether baseline distribution is computed automatically on feast apply. Defaults to true. |
122119

123120

124-
#### DataQualityMonitoringConfig
121+
#### DefaultCtrConfigs
125122

126123

127124

128-
DataQualityMonitoringConfig defines the Data Quality Monitoring configuration.
125+
DefaultCtrConfigs k8s container settings that are applied by default
129126

130127
_Appears in:_
131-
- [FeatureStoreSpec](#featurestorespec)
128+
- [ContainerConfigs](#containerconfigs)
129+
- [CronJobContainerConfigs](#cronjobcontainerconfigs)
130+
- [RegistryServerConfigs](#registryserverconfigs)
131+
- [ServerConfigs](#serverconfigs)
132132

133133
| Field | Description |
134134
| --- | --- |
135-
| `autoBaseline` _boolean_ | AutoBaseline controls whether baseline distribution is computed automatically on feast apply. Defaults to true. |
135+
| `image` _string_ | |
136136

137137

138138
#### FeastCronJob
@@ -289,7 +289,7 @@ _Appears in:_
289289
| `authz` _[AuthzConfig](#authzconfig)_ | |
290290
| `cronJob` _[FeastCronJob](#feastcronjob)_ | |
291291
| `batchEngine` _[BatchEngineConfig](#batchengineconfig)_ | |
292-
| `dqm` _[DataQualityMonitoringConfig](#dataqualitymonitoringconfig)_ | DataQualityMonitoring configures Data Quality Monitoring behaviour. |
292+
| `dataQualityMonitoring` _[DataQualityMonitoringConfig](#dataqualitymonitoringconfig)_ | DataQualityMonitoring configures Data Quality Monitoring behaviour. |
293293
| `replicas` _integer_ | Replicas is the desired number of pod replicas. Used by the scale sub-resource.
294294
Mutually exclusive with services.scaling.autoscaling. |
295295
| `materialization` _[MaterializationConfig](#materializationconfig)_ | Materialization controls feature materialization behavior (batch size, pull strategy).

‎infra/feast-operator/internal/controller/authz/authz.go‎

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,11 @@ import (
1515
"sigs.k8s.io/controller-runtime/pkg/log"
1616
)
1717

18+
const (
19+
authenticationAPIGroup = "authentication.k8s.io"
20+
verbCreate = "create"
21+
)
22+
1823
// Deploy the feast authorization
1924
func (authz *FeastAuthorization) Deploy() error {
2025
if authz.isKubernetesAuth() {
@@ -152,32 +157,32 @@ func (authz *FeastAuthorization) setFeastClusterRole(clusterRole *rbacv1.Cluster
152157
{
153158
APIGroups: []string{rbacv1.GroupName},
154159
Resources: []string{"rolebindings"},
155-
Verbs: []string{"list"},
160+
Verbs: []string{verbList},
156161
},
157162
{
158-
APIGroups: []string{"authentication.k8s.io"},
159-
Resources: []string{"tokenreviews"},
160-
Verbs: []string{"create"},
163+
APIGroups: []string{authenticationAPIGroup},
164+
Resources: []string{resourceTokenReviews},
165+
Verbs: []string{verbCreate},
161166
},
162167
{
163168
APIGroups: []string{rbacv1.GroupName},
164169
Resources: []string{"subjectaccessreviews"},
165-
Verbs: []string{"create"},
170+
Verbs: []string{verbCreate},
166171
},
167172
{
168173
APIGroups: []string{""},
169174
Resources: []string{"namespaces"},
170-
Verbs: []string{"get", "list", "watch"},
175+
Verbs: []string{verbGet, verbList, verbWatch},
171176
},
172177
{
173178
APIGroups: []string{rbacv1.GroupName},
174179
Resources: []string{"clusterroles"},
175-
Verbs: []string{"get", "list"},
180+
Verbs: []string{verbGet, verbList},
176181
},
177182
{
178183
APIGroups: []string{rbacv1.GroupName},
179184
Resources: []string{"clusterrolebindings"},
180-
Verbs: []string{"get", "list"},
185+
Verbs: []string{verbGet, verbList},
181186
},
182187
}
183188
// Don't set controller reference for shared ClusterRole
@@ -238,32 +243,32 @@ func (authz *FeastAuthorization) setFeastRole(role *rbacv1.Role) error {
238243
{
239244
APIGroups: []string{rbacv1.GroupName},
240245
Resources: []string{"roles", "rolebindings"},
241-
Verbs: []string{"get", "list", "watch"},
246+
Verbs: []string{verbGet, verbList, verbWatch},
242247
},
243248
{
244-
APIGroups: []string{"authentication.k8s.io"},
245-
Resources: []string{"tokenreviews"},
246-
Verbs: []string{"create"},
249+
APIGroups: []string{authenticationAPIGroup},
250+
Resources: []string{resourceTokenReviews},
251+
Verbs: []string{verbCreate},
247252
},
248253
{
249254
APIGroups: []string{rbacv1.GroupName},
250255
Resources: []string{"subjectaccessreviews"},
251-
Verbs: []string{"create"},
256+
Verbs: []string{verbCreate},
252257
},
253258
{
254259
APIGroups: []string{""},
255260
Resources: []string{"namespaces"},
256-
Verbs: []string{"get", "list", "watch"},
261+
Verbs: []string{verbGet, verbList, verbWatch},
257262
},
258263
{
259264
APIGroups: []string{rbacv1.GroupName},
260265
Resources: []string{"clusterroles"},
261-
Verbs: []string{"get", "list"},
266+
Verbs: []string{verbGet, verbList},
262267
},
263268
{
264269
APIGroups: []string{rbacv1.GroupName},
265270
Resources: []string{"clusterrolebindings"},
266-
Verbs: []string{"get", "list"},
271+
Verbs: []string{verbGet, verbList},
267272
},
268273
}
269274

@@ -347,9 +352,9 @@ func (authz *FeastAuthorization) createOidcClusterRole() error {
347352
clusterRole.Labels = authz.getSharedOidcClusterRoleLabels()
348353
clusterRole.Rules = []rbacv1.PolicyRule{
349354
{
350-
APIGroups: []string{"authentication.k8s.io"},
351-
Resources: []string{"tokenreviews"},
352-
Verbs: []string{"create"},
355+
APIGroups: []string{authenticationAPIGroup},
356+
Resources: []string{resourceTokenReviews},
357+
Verbs: []string{verbCreate},
353358
},
354359
}
355360
return nil
@@ -432,7 +437,7 @@ func (authz *FeastAuthorization) setFeastOidcAuthCondition(err error) error {
432437
if err != nil {
433438
logger := log.FromContext(authz.Handler.Context)
434439
cond := feastOidcAuthConditions[metav1.ConditionFalse]
435-
cond.Message = "Error: " + err.Error()
440+
cond.Message = services.ErrorMessagePrefix + err.Error()
436441
apimeta.SetStatusCondition(&authz.Handler.FeatureStore.Status.Conditions, cond)
437442
logger.Error(err, "Error deploying the OIDC authorization")
438443
return err
@@ -445,13 +450,12 @@ func (authz *FeastAuthorization) setFeastKubernetesAuthCondition(err error) erro
445450
if err != nil {
446451
logger := log.FromContext(authz.Handler.Context)
447452
cond := feastKubernetesAuthConditions[metav1.ConditionFalse]
448-
cond.Message = "Error: " + err.Error()
453+
cond.Message = services.ErrorMessagePrefix + err.Error()
449454
apimeta.SetStatusCondition(&authz.Handler.FeatureStore.Status.Conditions, cond)
450455
logger.Error(err, "Error deploying the Kubernetes authorization")
451456
return err
452-
} else {
453-
apimeta.SetStatusCondition(&authz.Handler.FeatureStore.Status.Conditions, feastKubernetesAuthConditions[metav1.ConditionTrue])
454457
}
458+
apimeta.SetStatusCondition(&authz.Handler.FeatureStore.Status.Conditions, feastKubernetesAuthConditions[metav1.ConditionTrue])
455459
return nil
456460
}
457461

0 commit comments

Comments
 (0)