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

Support x-ms-mutability #239

Closed
jaredmoo opened this issue Apr 27, 2018 · 5 comments
Closed

Support x-ms-mutability #239

jaredmoo opened this issue Apr 27, 2018 · 5 comments

Comments

@jaredmoo
Copy link

In sql jobs.json we have a 'credential' type with property 'password' that is required for PUT requests but always null in responses (intentionally in order to protect the secret value). Therefore we have mutability set to create/update.

https://github.com/azure/azure-rest-api-specs/blob/master/specification/sql/resource-manager/Microsoft.Sql/preview/2017-03-01-preview/jobs.json#L1953

    "JobCredentialProperties": {
      "description": "Properties of a job credential.",
      "required": [
        "username",
        "password"
      ],
      "type": "object",
      "properties": {
        "username": {
          "description": "The credential user name.",
          "type": "string"
        },
        "password": {
          "description": "The credential password.",
          "type": "string",
          "x-ms-mutability": [
            "create",
            "update"
          ]
        }
      }
    },

Our example files correctly have "password": null in responses. Model validator is giving an error for this. I believe the error is incorrect.

https://travis-ci.org/Azure/azure-rest-api-specs/jobs/369280815#L824

{ code: 'RESPONSE_VALIDATION_ERROR',
  id: 'OAV108',
  message: 'Found errors in validating the response with statusCode "200" for x-ms-example "Get a credential" in operation "JobCredentials_Get".',
  innerErrors: 
   [ { code: 'INVALID_RESPONSE_BODY',
       errors: 
        [ { code: 'ONE_OF_MISSING',
            params: [],
            message: 'Data does not match any schemas from \'oneOf\'',
            path: [ 'properties' ],
            inner: 
             [ { code: 'OBJECT_MISSING_REQUIRED_PROPERTY',
                 params: [ 'password' ],
                 message: 'Missing required property: password',
                 path: [ 'properties' ],
                 description: 'Properties of a job credential.' },
               { code: 'INVALID_TYPE',
                 params: [ 'null', 'object' ],
                 message: 'Expected type null but found type object',
                 path: [ 'properties' ] } ] } ],
       message: 'Invalid body: Data does not match any schemas from \'oneOf\'',
       path: [] } ] }
@amarzavery
Copy link
Contributor

At present oav does not understand x-ms-mutability extension.

However, if password can have a null value then shouldn't password be marked as "x-nullable": true in the spec like the way you do it for other cases in the sql swagger spec. oav has support for that. It would do the correct validation the way it does for other nullable types in sql.

@jaredmoo
Copy link
Author

Password must have a non-null value for requests, and must have a null value for responses. Would x-nullable be semantically correct in this case? If yes, we can do that.

@amarzavery
Copy link
Contributor

Oh I see.. That makes perfect sense.. We will have to add support to oav to understand x-ms-mutability in that case. That would be super useful during live validation as well.

/cc @vladbarosan, @salameer - This would be a very good feature to support. It will improve model and live validation checking. This is a fairly complex feature to implement. While implementing this we need to make sure how would x-nullable and x-ms-mutability play together.

@jaredmoo
Copy link
Author

jaredmoo commented Apr 27, 2018

What's your recommendation for now?
/cc @johnpaulkee

@amarzavery
Copy link
Contributor

Reference this issue in the swagger PR and move forward. We shall triage this issue and prioritize this in the upcoming Sprint.

@vladbarosan vladbarosan changed the title Model validator gives warning when a required write-only property has null in response. Support x-ms-mutability May 1, 2018
@vladbarosan vladbarosan self-assigned this May 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants