-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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 and Examples for the new Scoped tokens feature of Azure Container registry #5543
Conversation
Automation for azure-sdk-for-goNothing to generate for azure-sdk-for-go |
Automation for azure-sdk-for-pythonThe initial PR has been merged into your service PR: |
Automation for azure-sdk-for-rubyEncountered a Subprocess error: (azure-sdk-for-ruby)
Command: ['/usr/local/bin/autorest', '/tmp/tmpnds_adqx/rest/specification/containerregistry/resource-manager/readme.md', '--perform-load=false', '--swagger-to-sdk', '--output-artifact=configuration.json', '--input-file=foo', '--output-folder=/tmp/tmpem2k9nx3'] AutoRest code generation utility [version: 2.0.4283; node: v8.12.0]
(C) 2018 Microsoft Corporation.
https://aka.ms/autorest
Failure:
Error: Unable to start AutoRest Core from /root/.autorest/@[email protected]/node_modules/@microsoft.azure/autorest-core
Error: Unable to start AutoRest Core from /root/.autorest/@[email protected]/node_modules/@microsoft.azure/autorest-core
at main (/opt/node_modules/autorest/dist/app.js:232:19)
at <anonymous>
/root/.autorest/@[email protected]/node_modules/@microsoft.azure/autorest-core/dist/app.js:33
autorest_core_1.Shutdown();
^
ReferenceError: autorest_core_1 is not defined
at process.on (/root/.autorest/@[email protected]/node_modules/@microsoft.azure/autorest-core/dist/app.js:33:5)
at emitOne (events.js:121:20)
at process.emit (events.js:211:7)
at process.emit (/node_modules/source-map-support/source-map-support.js:439:21)
fs.js:612
return binding.close(fd);
^
Error: EBADF: bad file descriptor, close
at Object.fs.closeSync (fs.js:612:18)
at StaticVolumeFile.shutdown (/opt/node_modules/autorest/dist/static-loader.js:352:10)
at StaticFilesystem.shutdown (/opt/node_modules/autorest/dist/static-loader.js:406:17)
at process.exit.n [as exit] (/opt/node_modules/autorest/dist/static-loader.js:169:11)
at printErrorAndExit (/node_modules/source-map-support/source-map-support.js:423:11)
at process.emit (/node_modules/source-map-support/source-map-support.js:435:16)
at process._fatalException (bootstrap_node.js:391:26) |
Automation for azure-sdk-for-jsThe initial PR has been merged into your service PR: |
Automation for azure-sdk-for-javaThe initial PR has been merged into your service PR: |
Can one of the admins verify this patch? |
REST Spec PR 'Azure/azure-rest-api-specs#5543' REST Spec PR Author 'mnltejaswini' REST Spec PR Last commit
Automation for azure-sdk-for-netA PR has been created for you: |
REST Spec PR 'Azure/azure-rest-api-specs#5543' REST Spec PR Author 'mnltejaswini' REST Spec PR Last commit
If you haven't taken a look at our new onboarding experience at OpenAPIHub, it's a convenient way to create your PR when you're copying from an existing API version or when you're editing your existing specs. If you have any feedback or questions, feel free to use the feedback button on top of the site for help. Thanks! Also, regarding "[Do not merge]" tag, take a look at GitHub pull requests drafts - https://github.blog/2019-02-14-introducing-draft-pull-requests/. |
...nager/Microsoft.ContainerRegistry/preview/2019-05-01-preview/containerregistry_scopemap.json
Show resolved
Hide resolved
...nager/Microsoft.ContainerRegistry/preview/2019-05-01-preview/containerregistry_scopemap.json
Outdated
Show resolved
Hide resolved
...nager/Microsoft.ContainerRegistry/preview/2019-05-01-preview/containerregistry_scopemap.json
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few small comments to review
"$ref": "#/definitions/ScopeMap" | ||
} | ||
}, | ||
"201": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A 202 would be more accurate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for PUT /PATCH resource we are following the AzureAsync header pattern rather than Location header pattern as recommended by https://armwiki.azurewebsites.net/faq/Intro.html?q=async
"$ref": "#/definitions/Token" | ||
} | ||
}, | ||
"201": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
202?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
"type": "string" | ||
}, | ||
"name": { | ||
"description": "Specifies name of the password which should be regenerated if any -- password or password1.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The description doesn't match the enum
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed it.
"type": "string", | ||
"x-ms-enum": { | ||
"name": "PasswordName", | ||
"modelAsString": false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
modelAsString: true would allow for future enum additions without breaking API
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed it
"type": "string", | ||
"x-ms-enum": { | ||
"name": "PasswordName", | ||
"modelAsString": false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See prior comment on modelAsString: true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed it
} | ||
} | ||
}, | ||
"ProxyResource": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI, there are common definitions you can reference in: https://github.com/Azure/azure-rest-api-specs/blob/master/specification/common-types/resource-management/v1/types.json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@KrisBash The parent swagger has some description mismatch with that defined in common types. The build breaks for that. I will fix the parent swagger and this change separate to this PR. For now I added the custom proxy resource schema to the definitions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a recommendation, not blocking. thanks
@KrisBash Addressed your feedback. |
...nager/Microsoft.ContainerRegistry/preview/2019-05-01-preview/containerregistry_scopemap.json
Show resolved
Hide resolved
@kpajdzik We will be rolling out RP changes to Production starting next week after Build. Once it si deployed, I will resolve conflicts and remove the tag Do not Update tag on the title. |
REST Spec PR 'Azure/azure-rest-api-specs#5543' REST Spec PR Author 'mnltejaswini' REST Spec PR Last commit
@mnltejaswini, is it ready to merge? |
@kpajdzik Thank you for checking on it. The rollout is in progress. Its start was delayed due to some internal reasons. By next week, the rollout should be completed. |
REST Spec PR 'Azure/azure-rest-api-specs#5543' REST Spec PR Author 'mnltejaswini' REST Spec PR Last commit
Not sure why the SDK python failed. It complains INFO:swaggertosdk.autorest_tools: File "./scripts/multiapi_init_gen.py", line 22, in |
I don't think you should be worried. @lmazuel to confirm. |
@kpajdzik we have rolled out to the region where this feature will be enabled. If you reviewed it, it is good for merging. |
@sergey-shandar - this is ready to merge |
@lmazuel could you have a look why Python is failing? |
* .NET SDK Resource Provider:'ContainerRegistry' REST Spec PR 'Azure/azure-rest-api-specs#5543' REST Spec PR Author 'mnltejaswini' REST Spec PR Last commit * .NET SDK Resource Provider:'ContainerRegistry' REST Spec PR 'Azure/azure-rest-api-specs#5543' REST Spec PR Author 'mnltejaswini' REST Spec PR Last commit
…6161) * .NET SDK Resource Provider:'ContainerRegistry' REST Spec PR 'Azure/azure-rest-api-specs#5543' REST Spec PR Author 'mnltejaswini' REST Spec PR Last commit * .NET SDK Resource Provider:'ContainerRegistry' REST Spec PR 'Azure/azure-rest-api-specs#5543' REST Spec PR Author 'mnltejaswini' REST Spec PR Last commit
Latest improvements:
MSFT employees can try out our new experience at OpenAPI Hub - one location for using our validation tools and finding your workflow.
Contribution checklist:
ARM API Review Checklist
Failure to comply may result in delays for manifest application. Note this does not apply to data plane APIs.
Please follow the link to find more details on API review process.
Feature Description
Scoped tokens gives ability for users to create tokens with fine grained scope to the artifacts of the registry. Registry administrator defines the permission maps/sets by creating scope maps. After the permission sets are defined by creating scope maps, tokens can be created for a registry with specific scope map. Users can generate credentials for a token and the permissions of the credentials are bound to a token and thereby transitively bound to the scope map.
We are adding two new resources "scopeMaps" and "tokens" under the top level resource "registries". ScopeMaps model the permission sets and tokens are the mapping to the specific scope map.