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

Swagger ModelValidation erroneously fails when required x-ms-secret properties are omitted from responses in examples #20235

Open
erjms opened this issue Aug 15, 2022 · 4 comments
Assignees

Comments

@erjms
Copy link
Member

erjms commented Aug 15, 2022

From the x-ms-secret documentation:

When a property is modeled as both "required" and "x-ms-secret": true, which means that this property must not exist in the response but has to be present in request.

The Swagger ModelValidation pipeline fails for this scenario, erroneously flagging properties that match this scenario that are missing from example responses as errors, e.g.:
https://github.com/Azure/azure-rest-api-specs/pull/18645/checks?check_run_id=6586657182

@ghost ghost added the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Aug 15, 2022
@raych1
Copy link
Member

raych1 commented Aug 18, 2022

@keryul , can you check if this issue was fixed?

@raych1 raych1 removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Aug 18, 2022
@ankhyk
Copy link
Contributor

ankhyk commented Aug 19, 2022

hi @erjms , from the example of x-ms-secret documentation

"SecretAuthInfo": {
      "type": "object",
      "description": "The authentication info when authType is secret",
      "properties": {
        "name": {
          "description": "Username or account name for secret auth.",
          "type": "string"
        },
        "secret": {
          "description": "Password or account key for secret auth.",
          "type": "string",
          "x-ms-secret": true
        }
    }
}

x-ms-secret will not work when property has "$ref"

I suggest the following changes to swagger and example
swagger file

@@ -5337,12 +5337,7 @@
       "properties": {
         "secrets": {
           "description": "[Required] Storage account secrets.",
-          "$ref": "#/definitions/AccountKeyDatastoreSecrets",
-          "x-ms-mutability": [
-            "create",
-            "update"
-          ],
-          "x-ms-secret": true
+          "$ref": "#/definitions/AccountKeyDatastoreSecrets"
         }
       },
       "x-ms-discriminator-value": "AccountKey",
@@ -5360,6 +5355,11 @@
         "key": {
           "description": "Storage account key.",
           "type": "string",
+          "x-ms-mutability": [
+            "create",
+            "update"
+          ],
+          "x-ms-secret": true,
           "x-nullable": true
         }
       },

example file

@@ -31,7 +31,10 @@
               "isDefault": false,
               "properties": null,
               "credentials": {
-                "credentialsType": "AccountKey"
+                "credentialsType": "AccountKey",
+                "secrets": {
+                  "secretsType": "AccountKey"
+                }
               },
               "datastoreType": "AzureBlob",
               "accountName": "string",

@erjms
Copy link
Member Author

erjms commented Aug 29, 2022

Hi @keryul, thanks for responding. We are unable to make the suggested changes because our service's swagger has already been published as a stable version and they would constitute breaking changes. But more generally, could you please point me to where in the documentation it forbids applying x-ms-secret to $ref properties? I don't see any such restriction called out, and I can imagine many scenarios (such as ours) where we would need to mark an entire complex object as a secret, not just individual string properties.

@ankhyk
Copy link
Contributor

ankhyk commented Sep 7, 2022

Hi @erjms , we can find the relationship between x-ms-secret and x-ms-mutability in the fourth rule of x-ms-secret

"x-ms-secret": true is equivalent to "x-ms-mutability": ["create", "update"]

then we can find the last rule of x-ms-mutability

When this extension is applied on a collection (array, dictionary) then this will have effects on the mutability (adding/removing elements) of the collection. Mutability of the collection cannot be applied on its elements. The mutability of the element will be governed based on the mutability defined in the element's definition.

"The mutability of the element will be governed based on the mutability defined in the element's definition."
and the example "Mutability of the object property; which is a collection of items"

"definitions": {
  "ResourceCollection": {
    "description": "Collection of Resource objects. Resource is defined in the above example.",
    "properties": {
      "value": {
        "type": "array",
        "description": "Array of Resource objects.",
        "x-ms-mutability": ["create", "read", "update"], //This means that the array is mutable
        "items": {
          "type": object,
          "x-ms-mutability": ["create", "read"] // X - Applying mutability on the itemType of the array or valueType of the dictionary is not allowed.
          "schema": {
            "$ref": "#/definitions/Resource" // The mutability of the properties of the Resource object is governed by the mutability defined in it's model definition.
          }
        }
      }
    }
  }
}

"X - Applying mutability on the itemType of the array or valueType of the dictionary is not allowed."

A more convenient way is to imagine every property has its own x-ms-secret, with a default value of false.
the second rule of x-ms-secret:

When applying this extensions as: "x-ms-secret": false, it has same effect as not applying it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants