Skip to content
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

Policy Templates - chargingSpecification.sponStatus - Invalid values are ignored #123

Closed
dsilhavy opened this issue Jan 19, 2024 · 8 comments · Fixed by #130
Closed

Policy Templates - chargingSpecification.sponStatus - Invalid values are ignored #123

dsilhavy opened this issue Jan 19, 2024 · 8 comments · Fixed by #130
Assignees
Labels
bug Something isn't working

Comments

@dsilhavy
Copy link
Contributor

Description

This is more a question than an issue. According to TS29514_Npcf_PolicyAuthorization the SponsoringStatus type that is used for chargingSpecification.sponStatus shall have an enum value, namely either SPONSOR_DISABLED or SPONSOR_ENABLED.

The AF is not accepting any other values than these two. However, it also does not throw an error when an invalid value like the following is provided.

    "chargingSpecification": {
        "sponId": "sponId",
        "sponStatus": "SPONSOR_ENABLED_INVALID"    
    }

The configuration afterwards looks like this:

    "chargingSpecification": {
        "sponId": "sponId"
    }

The return code is 204 in this case. According to TS26512_M1_PolicyTemplatesProvisioning.yaml only two return codes are possible, namely 204 and 404. So I assume the current behavior is correct, although I find it irritating that no error is provided and the invalid field is simply omitted.

@dsilhavy dsilhavy added the question Further information is requested label Jan 19, 2024
@rjb1000
Copy link
Contributor

rjb1000 commented Jan 19, 2024

I agree that if the input is invalid, the Policy Template should not be accepted into the system and the HTTP response code should not be 204 No Content.

404 Not Found isn't really an appropriate error response. I would recommend 400 Bad Request, which is what will be specified in TS 26.510 Rel-18.

@davidjwbbc
Copy link
Contributor

The input isn't actually invalid as "SPONSOR_ENABLED_INVALID" could be a future value. It isn't ideal that the field is just ignored without warning, but it is sort of correct behaviour... difficult to know what's the best course of action here.

@dsilhavy
Copy link
Contributor Author

The input isn't actually invalid as "SPONSOR_ENABLED_INVALID" could be a future value. It isn't ideal that the field is just ignored without warning, but it is sort of correct behaviour... difficult to know what's the best course of action here.

To clarify, my original issue description wasn't correct. TS29514_Npcf_PolicyAuthorization.SponsoringStatus allows any string value:

   SponsoringStatus:
      description: Indicates whether sponsored data connectivity is enabled or disabled/not enabled.
      anyOf:
      - type: string
        enum:
          - SPONSOR_DISABLED
          - SPONSOR_ENABLED
      - type: string

Still, in my opinion, it would be reasonable to return an error for a value that our implementation is currently not supporting. If in the future we are supporting other values the implementation would need to change accordingly. Return code 422 might be an option for the current scenario: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/422

@rjb1000
Copy link
Contributor

rjb1000 commented Jan 22, 2024

My reading of 422 Unprocessable Content is that is applies in cases where the syntax is valid but the semantic is not.

This case falls in the crack between a syntax error and a semantic error. While the invalid enumerated value is technically valid in the the OpenAPI YAML syntax, our particular implementation doesn't support it because it is not a valid value in the current version of the specification. To my mind that feels more on the side of a syntax error, but one that isn't policed automatically by the OpenAPI YAML definition.

I would therefore lean slightly more towards a 400 Bad Request client error response code in this case.

@davidjwbbc
Copy link
Contributor

We can fix this in the openapi-generator template to fail parsing if the parsed value for any enumeration is unknown. This will cause any API interface where an enumeration is used to fail parsing and return an error.

@davidjwbbc
Copy link
Contributor

So this is the effect of making the template generate a parse error if the enumeration is not recognised.

$ curl -vv -H 'User-Agent: AF' -H 'Content-Type: application/json' -X PUT -d @PolicyTemplate-bad-sponsor-status.json http://127.0.0.22:7778/3gpp-m1/v2/provisioning-sessions/$psid/policy-templates/$polid; echo ''
> PUT /3gpp-m1/v2/provisioning-sessions/8e602d3e-b9ec-41ee-b17c-8959da210bdd/policy-templates/942c995a-b9ec-41ee-b17c-8959da210bdd HTTP/1.1
> Host: 127.0.0.22:7778
> Accept: */*
> User-Agent: AF
> Content-Type: application/json
> Content-Length: 467
> 
< HTTP/1.1 400 Bad Request
< Date: Tue, 23 Jan 2024 12:40:39 GMT
< Server: 5GMSAF-localhost/17 (info.title=M1_PolicyTemplatesProvisioning; info.version=2.2.1) rt-5gms-application-function/1.4.0
< Content-Type: application/problem+json
< Content-Length: 350
< 
{
	"type":	"/3gpp-m1/v2",
	"title":	"Updating policy template failed.",
	"status":	400,
	"detail":	"Updating policy template: Could not parse request body as JSON: Field \"sponStatus\" enumeration value not recognised",
	"instance":	"/provisioning-sessions/8e602d3e-b9ec-41ee-b17c-8959da210bdd/policy-templates/942c995a-b9ec-41ee-b17c-8959da210bdd"
}

So a 400 error is now generated when the enumerated value is not a recognised enumeration string.

@davidjwbbc davidjwbbc linked a pull request Jan 23, 2024 that will close this issue
@davidjwbbc davidjwbbc added bug Something isn't working and removed question Further information is requested labels Jan 23, 2024
@dsilhavy
Copy link
Contributor Author

Can confirm that the following error is thrown when trying to update sponStatus with a value that is not included in the enumeration:

{
    "type": "/3gpp-m1/v2",
    "title": "Updating policy template failed.",
    "status": 400,
    "detail": "Updating policy template: Could not parse request body as JSON: Field \"sponStatus\" enumeration value not recognised",
    "instance": "/provisioning-sessions/e47bf3a4-b9f3-41ee-85c0-59d1012fc832/policy-templates/f3c3bfb8-b9f3-41ee-85c0-59d1012fc832"
}

@davidjwbbc
Copy link
Contributor

Closed by PR #130

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging a pull request may close this issue.

4 participants