From: srinivasyanamadala Date: Wed, 4 Jun 2025 12:45:26 +0000 (+0200) Subject: opa-pdp remove check for non mandatory parameters in data and decision requests X-Git-Tag: 1.0.6^0 X-Git-Url: https://gerrit.onap.org/r/gitweb?a=commitdiff_plain;h=refs%2Fchanges%2F15%2F141115%2F4;p=policy%2Fopa-pdp.git opa-pdp remove check for non mandatory parameters in data and decision requests Issue-ID: POLICY-5379 Change-Id: I7ea55396d6a163dbdd8eda9bece5ede3c69c0fd7 Signed-off-by: srinivasyanamadala --- diff --git a/api/openapi.yaml b/api/openapi.yaml index e448ab0..e2042a5 100644 --- a/api/openapi.yaml +++ b/api/openapi.yaml @@ -178,7 +178,7 @@ paths: tags: - Statistics summary: Fetch current statistics - description: Provides current statistics of the Policy OPA PDP component + description: Provides current statistics of the Policy OPA PDP component operationId: statistics parameters: - name: X-ONAP-RequestID @@ -454,7 +454,7 @@ components: user: alice action: read object: id123 - type: dog + type: human required: - policyName - policyFilter @@ -495,7 +495,10 @@ components: user: alice action: read object: id123 - type: dog + type: human + required: + - policyName + - data HealthCheckReport: type: object properties: @@ -520,10 +523,6 @@ components: output: type: object additionalProperties: true - required: - - policyName - - output - StatisticsReport: type: object properties: diff --git a/pkg/data/data-handler.go b/pkg/data/data-handler.go index 3267408..33714f1 100644 --- a/pkg/data/data-handler.go +++ b/pkg/data/data-handler.go @@ -144,13 +144,13 @@ func patchHandler(res http.ResponseWriter, req *http.Request) { data := requestBody.Data log.Infof("data : %s", data) policyId := requestBody.PolicyName - if policyId == nil { + if policyId == ""{ errMsg := "Policy Id is nil" sendErrorResponse(res, errMsg, http.StatusBadRequest) return } - log.Infof("policy name : %s", *policyId) - isExists := policymap.CheckIfPolicyAlreadyExists(*policyId) + log.Infof("policy name : %s", policyId) + isExists := policymap.CheckIfPolicyAlreadyExists(policyId) if !isExists { errMsg := "Policy associated with the patch request does not exists" sendErrorResponse(res, errMsg, http.StatusBadRequest) @@ -158,12 +158,12 @@ func patchHandler(res http.ResponseWriter, req *http.Request) { return } - matchFound := validatePolicyDataPathMatched(dirParts, *policyId, res) + matchFound := validatePolicyDataPathMatched(dirParts, policyId, res) if !matchFound { return } - patchInfos, err := getPatchInfo(requestBody.Data, dataDir, res) + patchInfos, err := getPatchInfo(&requestBody.Data, dataDir, res) if err != nil { log.Warnf("Failed to get Patch Info : %v", err) return @@ -207,7 +207,7 @@ func extractDataDir(req *http.Request) (string, []string) { func validateRequest(requestBody *oapicodegen.OPADataUpdateRequest) error { validationErrors := utils.ValidateOPADataRequest(requestBody) - if !utils.IsValidData(requestBody.Data) { + if !utils.IsValidData(&requestBody.Data) { validationErrors = append(validationErrors, "Data is required and cannot be empty") } if len(validationErrors) > 0 { diff --git a/pkg/data/data-handler_test.go b/pkg/data/data-handler_test.go index 6cd5e5c..bd6becc 100644 --- a/pkg/data/data-handler_test.go +++ b/pkg/data/data-handler_test.go @@ -99,8 +99,8 @@ func TestPatchHandlerWithInvalidData(t *testing.T) { OnapComponent: &onapComp, OnapInstance: &onapIns, OnapName: &onapName, - PolicyName: &policyName, - Data: &data, + PolicyName: policyName, + Data: data, } // Marshal the request to JSON @@ -145,8 +145,8 @@ func TestPatchHandlerWithInvalidPolicyId(t *testing.T) { OnapComponent: &onapComp, OnapInstance: &onapIns, OnapName: &onapName, - PolicyName: &policyName, - Data: &data, + PolicyName: policyName, + Data: data, } // Marshal the request to JSON requestBody, err := json.Marshal(validRequest) @@ -647,8 +647,8 @@ func TestPatchHandler_EmptyDataField(t *testing.T) { OnapComponent: &onapComp, OnapInstance: &onapIns, OnapName: &onapName, - PolicyName: &policyName, - Data: &data, // Empty data + PolicyName: policyName, + Data: data, // Empty data } // Marshal the request to JSON @@ -759,9 +759,8 @@ func TestPatchHandler_PolicyDoesNotExist(t *testing.T) { OnapComponent: &onapComp, OnapInstance: &onapIns, OnapName: &onapName, - - PolicyName: StringPointer("invalid-policy"), - Data: &[]map[string]interface{}{{"test": "value"}}, + PolicyName: "invalid-policy", + Data: []map[string]interface{}{{"test": "value"}}, } bodyBytes, _ := json.Marshal(requestBody) @@ -807,8 +806,8 @@ func TestPatchHandler_InvalidDataPath(t *testing.T) { OnapInstance: &onapIns, OnapName: &onapName, - PolicyName: StringPointer("valid-policy"), - Data: &[]map[string]interface{}{{"test": "value"}}, + PolicyName: "valid-policy", + Data: []map[string]interface{}{{"test": "value"}}, } bodyBytes, _ := json.Marshal(requestBody) diff --git a/pkg/decision/decision-provider.go b/pkg/decision/decision-provider.go index 3345e6f..35f8a6e 100644 --- a/pkg/decision/decision-provider.go +++ b/pkg/decision/decision-provider.go @@ -78,16 +78,16 @@ func writeErrorJSONResponse(res http.ResponseWriter, status int, errorDescriptio // creates a success decision response func createSuccessDecisionResponse(policyName string, output map[string]interface{}) *oapicodegen.OPADecisionResponse { return &oapicodegen.OPADecisionResponse{ - PolicyName: policyName, - Output: output, + PolicyName: &policyName, + Output: &output, } } // creates a decision response based on the provided parameters func createSuccessDecisionResponseWithStatus(policyName string, output map[string]interface{}, statusMessage string) *oapicodegen.OPADecisionResponse { return &oapicodegen.OPADecisionResponse{ - PolicyName: policyName, - Output: output, + PolicyName: &policyName, + Output: &output, StatusMessage: &statusMessage, } } @@ -136,6 +136,7 @@ func handleDecisionRequest(res http.ResponseWriter, req *http.Request, errorDtls return } + *policyId = decisionReq.PolicyName // Validate the request body validationErrors := utils.ValidateOPADataRequest(decisionReq) @@ -150,7 +151,6 @@ func handleDecisionRequest(res http.ResponseWriter, req *http.Request, errorDtls } log.Debugf("Validation successful for request fields") // If validation passes, handle the decision request - decisionReq.PolicyName = strings.ReplaceAll(decisionReq.PolicyName, ".", "/") handlePolicyValidation(res, decisionReq, errorDtls, httpStatus, policyId) } @@ -276,7 +276,7 @@ func processOpaDecision(res http.ResponseWriter, opa *sdk.OPA, decisionReq *oapi log.Warnf("Failed to unmarshal decision Request Input: %vg", err) return } - if inputBytes == nil || len(inputBytes) == 0 { + if inputBytes == nil || len(inputBytes) == 0 || string(inputBytes) == "null" { statusMessage := "{\"warning\":{\"code\":\"api_usage_warning\",\"message\":\"'input' key missing from the request\"}}" decisionRes = createSuccessDecisionResponseWithStatus(decisionReq.PolicyName, nil, statusMessage) } else { diff --git a/pkg/decision/decision-provider_test.go b/pkg/decision/decision-provider_test.go index 387d07a..abf517d 100644 --- a/pkg/decision/decision-provider_test.go +++ b/pkg/decision/decision-provider_test.go @@ -289,8 +289,8 @@ func ptrStringEx(s string) *string { } // Utility function to return a pointer to a map -func ptrMap(m map[string]interface{}) map[string]interface{} { - return m +func ptrMap(m map[string]interface{}) *map[string]interface{} { + return &m } // Utility function to return a pointer to a OPADecisionResponseDecision @@ -298,7 +298,7 @@ func TestWriteOpaJSONResponse(t *testing.T) { rec := httptest.NewRecorder() data := &oapicodegen.OPADecisionResponse{ - PolicyName: ptrString("test-policy"), + PolicyName: ptrStringEx("test-policy"), Output: ptrMap(map[string]interface{}{"key": "value"}), } @@ -336,10 +336,10 @@ func TestCreateSuccessDecisionResponse(t *testing.T) { // Assertions // Check the PolicyName field - assert.Equal(t, response.PolicyName, policyName, "PolicyName should match") + assert.Equal(t, *response.PolicyName, policyName, "PolicyName should match") // Check the Output field - assert.Equal(t, response.Output, output, "Output should match") + assert.Equal(t, *response.Output, output, "Output should match") } // Test for policy filter @@ -366,7 +366,7 @@ func TestWriteOpaJSONResponse_Error(t *testing.T) { // Create a response object for error scenario data := &oapicodegen.OPADecisionResponse{ - PolicyName: ptrString(policyName), + PolicyName: ptrStringEx(policyName), Output: ptrMap(output), } @@ -382,8 +382,8 @@ func TestWriteOpaJSONResponse_Error(t *testing.T) { func TestWriteOpaJSONResponse_Success(t *testing.T) { // Prepare test data decisionRes := oapicodegen.OPADecisionResponse{ - PolicyName: ptrString("TestPolicy"), - Output: map[string]interface{}{"key": "value"}, + PolicyName: ptrStringEx("TestPolicy"), + Output: &(map[string]interface{}{"key": "value"}), } // Create a mock HTTP response writer @@ -414,7 +414,7 @@ func TestWriteOpaJSONResponse_EncodingError(t *testing.T) { // Prepare invalid test data to trigger JSON encoding error decisionRes := oapicodegen.OPADecisionResponse{ // Introducing an invalid type to cause encoding failure - Output: map[string]interface{}{"key": make(chan int)}, + Output: &map[string]interface{}{"key": make(chan int)}, } // Create a mock HTTP response writer @@ -703,7 +703,7 @@ func Test_Invalid_Decision(t *testing.T) { // Call the handler function that processes OPA decision OpaDecision(res, req) // Assert that the response status code is 200 - assert.Equal(t, 500, res.Code) + assert.Equal(t, 200, res.Code) } // Test for Invalid Decision error in Decision Result diff --git a/pkg/model/oapicodegen/models.go b/pkg/model/oapicodegen/models.go index 59d9a74..852bf4b 100644 --- a/pkg/model/oapicodegen/models.go +++ b/pkg/model/oapicodegen/models.go @@ -66,14 +66,14 @@ type OPADataResponse_Data struct { // OPADataUpdateRequest defines model for OPADataUpdateRequest. type OPADataUpdateRequest struct { - CurrentDate *openapi_types.Date `json:"currentDate,omitempty"` - CurrentDateTime *time.Time `json:"currentDateTime,omitempty"` - CurrentTime *string `json:"currentTime,omitempty"` - Data *[]map[string]interface{} `json:"data,omitempty"` - OnapComponent *string `json:"onapComponent,omitempty"` - OnapInstance *string `json:"onapInstance,omitempty"` - OnapName *string `json:"onapName,omitempty"` - PolicyName *string `json:"policyName,omitempty"` + CurrentDate *openapi_types.Date `json:"currentDate,omitempty"` + CurrentDateTime *time.Time `json:"currentDateTime,omitempty"` + CurrentTime *string `json:"currentTime,omitempty"` + Data []map[string]interface{} `json:"data"` + OnapComponent *string `json:"onapComponent,omitempty"` + OnapInstance *string `json:"onapInstance,omitempty"` + OnapName *string `json:"onapName,omitempty"` + PolicyName string `json:"policyName"` // TimeOffset Time offset in hours and minutes, e.g., '+02:00' or '-05:00' TimeOffset *string `json:"timeOffset,omitempty"` @@ -114,9 +114,9 @@ type OPADecisionRequest_Input struct { // OPADecisionResponse defines model for OPADecisionResponse. type OPADecisionResponse struct { - Output map[string]interface{} `json:"output"` - PolicyName string `json:"policyName"` - StatusMessage *string `json:"statusMessage,omitempty"` + Output *map[string]interface{} `json:"output,omitempty"` + PolicyName *string `json:"policyName,omitempty"` + StatusMessage *string `json:"statusMessage,omitempty"` } // StatisticsReport defines model for StatisticsReport. diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index aa52503..d38718e 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -312,7 +312,7 @@ func IsValidData(data *[]map[string]interface{}) bool { // Custom validation function for CurrentDate func IsValidCurrentDate(currentDate *string) bool { if currentDate == nil || strings.TrimSpace(*currentDate) == "" { - return false + return true } re := regexp.MustCompile(`^\d{4}-\d{2}-\d{2}$`) // eg: "2025-01-17" return re.MatchString(*currentDate) @@ -396,7 +396,7 @@ func ValidateOPADataRequest(request interface{}) []string { OnapComponent: updateReq.OnapComponent, OnapInstance: updateReq.OnapInstance, OnapName: updateReq.OnapName, - PolicyName: convertPtrToString(updateReq.PolicyName), + PolicyName: updateReq.PolicyName, } return validateCommonFields(commonFields) @@ -439,29 +439,20 @@ func validateCommonFields(fields CommonFields) []string { var validationErrors []string - if fields.CurrentDate == nil || !IsValidCurrentDate(fields.CurrentDate) { - validationErrors = append(validationErrors, "CurrentDate is invalid or missing") - } - if fields.CurrentDateTime == nil || !IsValidTime(fields.CurrentDateTime) { - validationErrors = append(validationErrors, "CurrentDateTime is invalid or missing") - } - if fields.CurrentTime == nil || !IsValidCurrentTime(fields.CurrentTime) { - validationErrors = append(validationErrors, "CurrentTime is invalid or missing") - } - if fields.TimeOffset == nil || !IsValidTimeOffset(fields.TimeOffset) { - validationErrors = append(validationErrors, "TimeOffset is invalid or missing") + if fields.CurrentDate != nil && !IsValidCurrentDate(fields.CurrentDate) { + validationErrors = append(validationErrors, "CurrentDate is invalid") } - if fields.TimeZone == nil || !IsValidTimeZone(fields.TimeZone) { - validationErrors = append(validationErrors, "TimeZone is invalid or missing") + if fields.CurrentDateTime != nil && !IsValidTime(fields.CurrentDateTime) { + validationErrors = append(validationErrors, "CurrentDateTime is invalid") } - if !IsValidString(fields.OnapComponent) { - validationErrors = append(validationErrors, "OnapComponent is required") + if fields.CurrentTime != nil && !IsValidCurrentTime(fields.CurrentTime) { + validationErrors = append(validationErrors, "CurrentTime is invalid") } - if !IsValidString(fields.OnapInstance) { - validationErrors = append(validationErrors, "OnapInstance is required") + if fields.TimeOffset != nil && !IsValidTimeOffset(fields.TimeOffset) { + validationErrors = append(validationErrors, "TimeOffset is invalid") } - if !IsValidString(fields.OnapName) { - validationErrors = append(validationErrors, "OnapName is required") + if fields.TimeZone != nil && !IsValidTimeZone(fields.TimeZone) { + validationErrors = append(validationErrors, "TimeZone is invalid") } if !IsValidString(fields.PolicyName) { validationErrors = append(validationErrors, "PolicyName is required and cannot be empty") diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go index f6e4d29..623ff4a 100644 --- a/pkg/utils/utils_test.go +++ b/pkg/utils/utils_test.go @@ -533,8 +533,8 @@ func TestIsValidData(t *testing.T) { } func TestIsValidCurrentDate(t *testing.T) { - validDates := []string{"2025-01-17", "1999-12-31"} - invalidDates := []string{"20250117", "01-17-2025", "abcd-ef-gh", ""} + validDates := []string{"2025-01-17", "1999-12-31", ""} + invalidDates := []string{"20250117", "01-17-2025", "abcd-ef-gh"} for _, date := range validDates { if !IsValidCurrentDate(&date) { @@ -710,7 +710,7 @@ func TestValidateOPADataRequest_UpdateRequest_Valid(t *testing.T) { OnapComponent: strPtr("Component"), OnapInstance: strPtr("Instance"), OnapName: strPtr("Onap"), - PolicyName: strPtr("Policy1"), + PolicyName: "Policy1", } errors := ValidateOPADataRequest(updateReq)