Skip to content

Commit c0b293d

Browse files
mayank-devtronmayank-devtron
committed
refactor app list tag-filter validation
(cherry picked from commit 60ef7488f49db9b56e2f55323066cc8ea8ba2c55)
1 parent ab2c0e0 commit c0b293d

5 files changed

Lines changed: 96 additions & 39 deletions

File tree

‎api/restHandler/app/appList/AppListingRestHandler.go‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ import (
5656
"github.com/gorilla/mux"
5757
"go.opentelemetry.io/otel"
5858
"go.uber.org/zap"
59+
"gopkg.in/go-playground/validator.v9"
5960
"net/http"
6061
"strconv"
6162
"time"
@@ -98,6 +99,7 @@ type AppListingRestHandlerImpl struct {
9899
k8sApplicationService k8sApplication.K8sApplicationService
99100
deploymentConfigService common2.DeploymentConfigService
100101
resourceTreeService resourceTree.Service
102+
validator *validator.Validate
101103
}
102104

103105
type AppStatus struct {
@@ -141,6 +143,7 @@ func NewAppListingRestHandlerImpl(appListingService app.AppListingService,
141143
k8sApplicationService: k8sApplicationService,
142144
deploymentConfigService: deploymentConfigService,
143145
resourceTreeService: resourceTreeService,
146+
validator: validator.New(),
144147
}
145148
return appListingHandler
146149
}
@@ -276,6 +279,25 @@ func (handler AppListingRestHandlerImpl) FetchJobOverviewCiPipelines(w http.Resp
276279
common.WriteJsonResp(w, err, jobCi, http.StatusOK)
277280
}
278281

282+
// validateAndNormalizeFetchAppListingRequest applies request-level validation first,
283+
// then tag-filter business validation, and finally normalization.
284+
func (handler AppListingRestHandlerImpl) validateAndNormalizeFetchAppListingRequest(w http.ResponseWriter, r *http.Request, fetchAppListingRequest *app.FetchAppListingRequest) bool {
285+
err := handler.validator.Struct(*fetchAppListingRequest)
286+
if err != nil {
287+
handler.logger.Errorw("validation err, FetchAppsByEnvironment", "err", err, "payload", fetchAppListingRequest)
288+
common.HandleValidationErrors(w, r, err)
289+
return false
290+
}
291+
err = handler.appListingService.ValidateTagFilters(fetchAppListingRequest.TagFilters)
292+
if err != nil {
293+
handler.logger.Errorw("request err, ValidateTagFilters", "err", err, "payload", fetchAppListingRequest.TagFilters)
294+
common.WriteJsonResp(w, err, nil, http.StatusBadRequest)
295+
return false
296+
}
297+
fetchAppListingRequest.TagFilters = handler.appListingService.NormalizeTagFilters(fetchAppListingRequest.TagFilters)
298+
return true
299+
}
300+
279301
func (handler AppListingRestHandlerImpl) FetchAppsByEnvironmentV2(w http.ResponseWriter, r *http.Request) {
280302
//Allow CORS here By * or specific origin
281303
util3.SetupCorsOriginHeader(&w)
@@ -331,13 +353,9 @@ func (handler AppListingRestHandlerImpl) FetchAppsByEnvironmentV2(w http.Respons
331353
common.WriteJsonResp(w, err, nil, http.StatusBadRequest)
332354
return
333355
}
334-
normalizedTagFilters, err := app.NormalizeAndValidateTagFilters(fetchAppListingRequest.TagFilters)
335-
if err != nil {
336-
handler.logger.Errorw("request err, ValidateTagFilters", "err", err, "payload", fetchAppListingRequest.TagFilters)
337-
common.WriteJsonResp(w, err, nil, http.StatusBadRequest)
356+
if !handler.validateAndNormalizeFetchAppListingRequest(w, r, &fetchAppListingRequest) {
338357
return
339358
}
340-
fetchAppListingRequest.TagFilters = normalizedTagFilters
341359
newCtx, span = otel.Tracer("fetchAppListingRequest").Start(newCtx, "GetNamespaceClusterMapping")
342360
_, _, err = fetchAppListingRequest.GetNamespaceClusterMapping()
343361
span.End()

‎internal/sql/repository/helper/AppListingRepositoryQueryBuilder.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,8 @@ type TagFilterOperator string
6868
// value is required for EQUALS/DOES_NOT_EQUAL/CONTAINS/DOES_NOT_CONTAIN.
6969
// value must be absent for EXISTS/DOES_NOT_EXIST.
7070
type TagFilter struct {
71-
Key string `json:"key"`
72-
Operator TagFilterOperator `json:"operator"`
71+
Key string `json:"key" validate:"required"`
72+
Operator TagFilterOperator `json:"operator" validate:"required"`
7373
Value *string `json:"value"`
7474
}
7575

‎pkg/app/AppListingService.go‎

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@ type AppListingService interface {
7070
ISLastReleaseStopType(appId, envId int) (bool, error)
7171
ISLastReleaseStopTypeV2(pipelineIds []int) (map[int]bool, error)
7272
GetReleaseCount(appId, envId int) (int, error)
73+
ValidateTagFilters(tagFilters []helper.TagFilter) error
74+
NormalizeTagFilters(tagFilters []helper.TagFilter) []helper.TagFilter
7375

7476
FetchAppsByEnvironmentV2(fetchAppListingRequest FetchAppListingRequest, w http.ResponseWriter, r *http.Request, token string) ([]*AppView.AppEnvironmentContainer, int, error)
7577
FetchOverviewAppsByEnvironment(envId, limit, offset int) (*OverviewAppsByEnvironmentBean, error)
@@ -85,7 +87,7 @@ type FetchAppListingRequest struct {
8587
Environments []int `json:"environments"`
8688
Statuses []string `json:"statuses"`
8789
Teams []int `json:"teams"`
88-
TagFilters []helper.TagFilter `json:"tagFilters"`
90+
TagFilters []helper.TagFilter `json:"tagFilters" validate:"omitempty,dive"`
8991
AppNameSearch string `json:"appNameSearch"`
9092
SortOrder helper.SortOrder `json:"sortOrder"`
9193
SortBy helper.SortBy `json:"sortBy"`
@@ -128,38 +130,53 @@ func (req FetchAppListingRequest) GetNamespaceClusterMapping() (namespaceCluster
128130
return namespaceClusterPair, clusterIds, nil
129131
}
130132

131-
// NormalizeAndValidateTagFilters validates each tag filter row and normalizes value handling.
133+
// ValidateTagFilters validates each tag filter row.
132134
// Rules:
133135
// 1) key must be present.
134136
// 2) operator must be one of the supported backend operators.
135137
// 3) for EXISTS/DOES_NOT_EXIST, value must not be sent.
136138
// 4) for EQUALS/DOES_NOT_EQUAL/CONTAINS/DOES_NOT_CONTAIN, value must be non-empty.
137-
func NormalizeAndValidateTagFilters(tagFilters []helper.TagFilter) ([]helper.TagFilter, error) {
138-
if len(tagFilters) == 0 {
139-
return tagFilters, nil
140-
}
141-
normalizedFilters := make([]helper.TagFilter, 0, len(tagFilters))
139+
func ValidateTagFilters(tagFilters []helper.TagFilter) error {
142140
for index, tagFilter := range tagFilters {
143-
tagFilter.Key = strings.TrimSpace(tagFilter.Key)
144-
if len(tagFilter.Key) == 0 {
145-
return nil, fmt.Errorf("tagFilters[%d].key is required", index)
141+
if len(strings.TrimSpace(tagFilter.Key)) == 0 {
142+
return fmt.Errorf("tagFilters[%d].key is required", index)
146143
}
147144
if !tagFilter.Operator.IsValid() {
148-
return nil, fmt.Errorf("tagFilters[%d].operator is invalid: %s", index, tagFilter.Operator)
145+
return fmt.Errorf("tagFilters[%d].operator is invalid: %s", index, tagFilter.Operator)
149146
}
150147
switch tagFilter.Operator {
151148
case helper.TagFilterOperatorExists, helper.TagFilterOperatorDoesNotExist:
152149
if tagFilter.Value != nil {
153-
return nil, fmt.Errorf("tagFilters[%d].value must be empty for operator %s", index, tagFilter.Operator)
150+
return fmt.Errorf("tagFilters[%d].value must be empty for operator %s", index, tagFilter.Operator)
154151
}
155152
default:
156153
if tagFilter.Value == nil || len(*tagFilter.Value) == 0 {
157-
return nil, fmt.Errorf("tagFilters[%d].value is required for operator %s", index, tagFilter.Operator)
154+
return fmt.Errorf("tagFilters[%d].value is required for operator %s", index, tagFilter.Operator)
158155
}
159156
}
157+
}
158+
return nil
159+
}
160+
161+
// NormalizeTagFilters normalizes tag filter rows after validation.
162+
func NormalizeTagFilters(tagFilters []helper.TagFilter) []helper.TagFilter {
163+
if len(tagFilters) == 0 {
164+
return tagFilters
165+
}
166+
normalizedFilters := make([]helper.TagFilter, 0, len(tagFilters))
167+
for _, tagFilter := range tagFilters {
168+
tagFilter.Key = strings.TrimSpace(tagFilter.Key)
160169
normalizedFilters = append(normalizedFilters, tagFilter)
161170
}
162-
return normalizedFilters, nil
171+
return normalizedFilters
172+
}
173+
174+
func (impl AppListingServiceImpl) ValidateTagFilters(tagFilters []helper.TagFilter) error {
175+
return ValidateTagFilters(tagFilters)
176+
}
177+
178+
func (impl AppListingServiceImpl) NormalizeTagFilters(tagFilters []helper.TagFilter) []helper.TagFilter {
179+
return NormalizeTagFilters(tagFilters)
163180
}
164181

165182
type AppListingServiceImpl struct {

‎pkg/app/AppListingService_tag_filter_test.go‎

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -11,75 +11,86 @@ func strPointer(value string) *string {
1111
return &value
1212
}
1313

14-
func TestNormalizeAndValidateTagFilters_EqualsRequiresValue(t *testing.T) {
15-
_, err := NormalizeAndValidateTagFilters([]helper.TagFilter{
14+
func TestValidateTagFilters_EqualsRequiresValue(t *testing.T) {
15+
err := ValidateTagFilters([]helper.TagFilter{
1616
{Key: "owner", Operator: helper.TagFilterOperatorEquals, Value: nil},
1717
})
1818

1919
require.Error(t, err)
2020
require.Equal(t, "tagFilters[0].value is required for operator EQUALS", err.Error())
2121
}
2222

23-
func TestNormalizeAndValidateTagFilters_EqualsRejectsEmptyString(t *testing.T) {
24-
_, err := NormalizeAndValidateTagFilters([]helper.TagFilter{
23+
func TestValidateTagFilters_EqualsRejectsEmptyString(t *testing.T) {
24+
err := ValidateTagFilters([]helper.TagFilter{
2525
{Key: "owner", Operator: helper.TagFilterOperatorEquals, Value: strPointer("")},
2626
})
2727

2828
require.Error(t, err)
2929
require.Equal(t, "tagFilters[0].value is required for operator EQUALS", err.Error())
3030
}
3131

32-
func TestNormalizeAndValidateTagFilters_ContainsRequiresValue(t *testing.T) {
33-
_, err := NormalizeAndValidateTagFilters([]helper.TagFilter{
32+
func TestValidateTagFilters_ContainsRequiresValue(t *testing.T) {
33+
err := ValidateTagFilters([]helper.TagFilter{
3434
{Key: "owner", Operator: helper.TagFilterOperatorContains, Value: nil},
3535
})
3636

3737
require.Error(t, err)
3838
require.Equal(t, "tagFilters[0].value is required for operator CONTAINS", err.Error())
3939
}
4040

41-
func TestNormalizeAndValidateTagFilters_EmptyKeyReturnsError(t *testing.T) {
42-
_, err := NormalizeAndValidateTagFilters([]helper.TagFilter{
41+
func TestValidateTagFilters_EmptyKeyReturnsError(t *testing.T) {
42+
err := ValidateTagFilters([]helper.TagFilter{
4343
{Key: " ", Operator: helper.TagFilterOperatorEquals, Value: strPointer("James")},
4444
})
4545

4646
require.Error(t, err)
4747
require.Equal(t, "tagFilters[0].key is required", err.Error())
4848
}
4949

50-
func TestNormalizeAndValidateTagFilters_InvalidOperatorReturnsError(t *testing.T) {
51-
_, err := NormalizeAndValidateTagFilters([]helper.TagFilter{
50+
func TestValidateTagFilters_InvalidOperatorReturnsError(t *testing.T) {
51+
err := ValidateTagFilters([]helper.TagFilter{
5252
{Key: "owner", Operator: helper.TagFilterOperator("INVALID"), Value: strPointer("James")},
5353
})
5454

5555
require.Error(t, err)
5656
require.Equal(t, "tagFilters[0].operator is invalid: INVALID", err.Error())
5757
}
5858

59-
func TestNormalizeAndValidateTagFilters_ExistsAllowsNilValueOnly(t *testing.T) {
60-
normalizedFilters, err := NormalizeAndValidateTagFilters([]helper.TagFilter{
59+
func TestValidateTagFilters_ExistsAllowsNilValueOnly(t *testing.T) {
60+
err := ValidateTagFilters([]helper.TagFilter{
6161
{Key: "owner", Operator: helper.TagFilterOperatorExists, Value: nil},
6262
})
6363

6464
require.NoError(t, err)
65-
require.Len(t, normalizedFilters, 1)
66-
require.Nil(t, normalizedFilters[0].Value)
6765
}
6866

69-
func TestNormalizeAndValidateTagFilters_ExistsRejectsProvidedValue(t *testing.T) {
70-
_, err := NormalizeAndValidateTagFilters([]helper.TagFilter{
67+
func TestValidateTagFilters_ExistsRejectsProvidedValue(t *testing.T) {
68+
err := ValidateTagFilters([]helper.TagFilter{
7169
{Key: "owner", Operator: helper.TagFilterOperatorExists, Value: strPointer("James")},
7270
})
7371

7472
require.Error(t, err)
7573
require.Equal(t, "tagFilters[0].value must be empty for operator EXISTS", err.Error())
7674
}
7775

78-
func TestNormalizeAndValidateTagFilters_DoesNotExistRejectsProvidedValue(t *testing.T) {
79-
_, err := NormalizeAndValidateTagFilters([]helper.TagFilter{
76+
func TestValidateTagFilters_DoesNotExistRejectsProvidedValue(t *testing.T) {
77+
err := ValidateTagFilters([]helper.TagFilter{
8078
{Key: "owner", Operator: helper.TagFilterOperatorDoesNotExist, Value: strPointer("")},
8179
})
8280

8381
require.Error(t, err)
8482
require.Equal(t, "tagFilters[0].value must be empty for operator DOES_NOT_EXIST", err.Error())
8583
}
84+
85+
func TestNormalizeTagFilters_TrimsKey(t *testing.T) {
86+
filters := []helper.TagFilter{
87+
{Key: " owner ", Operator: helper.TagFilterOperatorEquals, Value: strPointer("James")},
88+
}
89+
90+
normalizedFilters := NormalizeTagFilters(filters)
91+
92+
require.Len(t, normalizedFilters, 1)
93+
require.Equal(t, "owner", normalizedFilters[0].Key)
94+
// Ensure input is not modified by normalization.
95+
require.Equal(t, " owner ", filters[0].Key)
96+
}

‎pkg/app/mocks/AppListingService.go‎

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

0 commit comments

Comments
 (0)