Skip to content

Conversation

@dandanwang0320
Copy link
Contributor

@dandanwang0320 dandanwang0320 commented May 8, 2019

Description:

Add schema of JIT request related APIs.
Most APIs are from AzureUX-ARM/src/providers/Roles/Providers.Feature/Controllers/JitRequestController.cs
And "updateAccess" api from
ARM/src/providers/Roles/Providers.Feature/Controllers/ApplicationApiController.cs

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:

  • I have reviewed the documentation for the workflow.
  • Validation tools were run on swagger spec(s) and have all been fixed in this PR.
  • The OpenAPI Hub was used for checking validation status and next steps.

ARM API Review Checklist

  • Service team MUST add the "WaitForARMFeedback" label if the management plane API changes fall into one of the below categories.
  • adding/removing APIs.
  • adding/removing properties.
  • adding/removing API-version.
  • adding a new service in Azure.

Failure to comply may result in delays for manifest application. Note this does not apply to data plane APIs.

  • If you are blocked on ARM review and want to get the PR merged urgently, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.
    Please follow the link to find more details on API review process.

@AutorestCI
Copy link

AutorestCI commented May 8, 2019

Automation for azure-sdk-for-ruby

Nothing to generate for azure-sdk-for-ruby

@AutorestCI
Copy link

AutorestCI commented May 8, 2019

Automation for azure-sdk-for-js

A PR has been created for you:
Azure/azure-sdk-for-js#3405

@AutorestCI
Copy link

AutorestCI commented May 8, 2019

Automation for azure-sdk-for-python

Encountered an unknown error: (azure-sdk-for-python)

Traceback (most recent call last):
  File "/usr/local/lib/python3.6/dist-packages/urllib3/connectionpool.py", line 384, in _make_request
    six.raise_from(e, None)
  File "<string>", line 2, in raise_from
  File "/usr/local/lib/python3.6/dist-packages/urllib3/connectionpool.py", line 380, in _make_request
    httplib_response = conn.getresponse()
  File "/usr/lib/python3.6/http/client.py", line 1331, in getresponse
    response.begin()
  File "/usr/lib/python3.6/http/client.py", line 297, in begin
    version, status, reason = self._read_status()
  File "/usr/lib/python3.6/http/client.py", line 258, in _read_status
    line = str(self.fp.readline(_MAXLINE + 1), "iso-8859-1")
  File "/usr/lib/python3.6/socket.py", line 586, in readinto
    return self._sock.recv_into(b)
  File "/usr/lib/python3.6/ssl.py", line 1012, in recv_into
    return self.read(nbytes, buffer)
  File "/usr/lib/python3.6/ssl.py", line 874, in read
    return self._sslobj.read(len, buffer)
  File "/usr/lib/python3.6/ssl.py", line 631, in read
    v = self._sslobj.read(len, buffer)
socket.timeout: The read operation timed out

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/lib/python3.6/dist-packages/requests/adapters.py", line 449, in send
    timeout=timeout
  File "/usr/local/lib/python3.6/dist-packages/urllib3/connectionpool.py", line 638, in urlopen
    _stacktrace=sys.exc_info()[2])
  File "/usr/local/lib/python3.6/dist-packages/urllib3/util/retry.py", line 367, in increment
    raise six.reraise(type(error), error, _stacktrace)
  File "/usr/local/lib/python3.6/dist-packages/urllib3/packages/six.py", line 686, in reraise
    raise value
  File "/usr/local/lib/python3.6/dist-packages/urllib3/connectionpool.py", line 600, in urlopen
    chunked=chunked)
  File "/usr/local/lib/python3.6/dist-packages/urllib3/connectionpool.py", line 386, in _make_request
    self._raise_timeout(err=e, url=url, timeout_value=read_timeout)
  File "/usr/local/lib/python3.6/dist-packages/urllib3/connectionpool.py", line 306, in _raise_timeout
    raise ReadTimeoutError(self, url, "Read timed out. (read timeout=%s)" % timeout_value)
urllib3.exceptions.ReadTimeoutError: HTTPSConnectionPool(host='api.github.com', port=443): Read timed out. (read timeout=15)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/lib/python3.6/dist-packages/azure_devtools/ci_tools/github_tools.py", line 33, in exception_to_github
    yield context
  File "/usr/local/lib/python3.6/dist-packages/swaggertosdk/restapi/github.py", line 170, in rest_handle_action
    return rest_pull_close(body, restapi_repo, sdk_pr_target_repo, sdkbase, sdk_tag)
  File "/usr/local/lib/python3.6/dist-packages/swaggertosdk/restapi/github.py", line 185, in rest_pull_close
    rest_pr_management(rest_pr, sdk_pr_target_repo, sdk_tag, sdk_default_base)
  File "/usr/local/lib/python3.6/dist-packages/swaggertosdk/restapi/github_handler.py", line 151, in rest_pr_management
    sdk_tag=sdk_tag
  File "/usr/local/lib/python3.6/dist-packages/swaggertosdk/SwaggerToSdkNewCLI.py", line 254, in generate_sdk_from_git_object
    with manage_git_folder(gh_token, Path(temp_dir) / Path("rest"), branched_rest_api_id, pr_number=pr_number) as restapi_git_folder, \
  File "/usr/lib/python3.6/contextlib.py", line 81, in __enter__
    return next(self.gen)
  File "/usr/local/lib/python3.6/dist-packages/azure_devtools/ci_tools/github_tools.py", line 272, in manage_git_folder
    clone_to_path(gh_token, temp_dir, split_git_id[0], branch_or_commit=branch, pr_number=pr_number)
  File "/usr/local/lib/python3.6/dist-packages/azure_devtools/ci_tools/github_tools.py", line 187, in clone_to_path
    login = user_from_token(gh_token).login
  File "/usr/local/lib/python3.6/dist-packages/github/AuthenticatedUser.py", line 229, in login
    self._completeIfNotSet(self._login)
  File "/usr/local/lib/python3.6/dist-packages/github/GithubObject.py", line 263, in _completeIfNotSet
    self._completeIfNeeded()
  File "/usr/local/lib/python3.6/dist-packages/github/GithubObject.py", line 267, in _completeIfNeeded
    self.__complete()
  File "/usr/local/lib/python3.6/dist-packages/github/GithubObject.py", line 272, in __complete
    self._url.value
  File "/usr/local/lib/python3.6/dist-packages/github/Requester.py", line 264, in requestJsonAndCheck
    return self.__check(*self.requestJson(verb, url, parameters, headers, input, self.__customConnection(url)))
  File "/usr/local/lib/python3.6/dist-packages/github/Requester.py", line 324, in requestJson
    return self.__requestEncode(cnx, verb, url, parameters, headers, input, encode)
  File "/usr/local/lib/python3.6/dist-packages/github/Requester.py", line 377, in __requestEncode
    status, responseHeaders, output = self.__requestRaw(cnx, verb, url, requestHeaders, encoded_input)
  File "/usr/local/lib/python3.6/dist-packages/github/Requester.py", line 401, in __requestRaw
    response = cnx.getresponse()
  File "/usr/local/lib/python3.6/dist-packages/github/Requester.py", line 109, in getresponse
    r = verb(url, headers=self.headers, data=self.input, timeout=self.timeout, verify=self.verify, allow_redirects=False)
  File "/usr/local/lib/python3.6/dist-packages/requests/sessions.py", line 546, in get
    return self.request('GET', url, **kwargs)
  File "/usr/local/lib/python3.6/dist-packages/requests/sessions.py", line 533, in request
    resp = self.send(prep, **send_kwargs)
  File "/usr/local/lib/python3.6/dist-packages/requests/sessions.py", line 646, in send
    r = adapter.send(request, **kwargs)
  File "/usr/local/lib/python3.6/dist-packages/requests/adapters.py", line 529, in send
    raise ReadTimeout(e, request=request)
requests.exceptions.ReadTimeout: HTTPSConnectionPool(host='api.github.com', port=443): Read timed out. (read timeout=15)

@AutorestCI
Copy link

AutorestCI commented May 8, 2019

Automation for azure-sdk-for-go

Encountered an unknown error: (azure-sdk-for-go)

Traceback (most recent call last):
  File "/usr/local/lib/python3.6/dist-packages/azure_devtools/ci_tools/github_tools.py", line 33, in exception_to_github
    yield context
  File "/usr/local/lib/python3.6/dist-packages/swaggertosdk/restapi/github.py", line 170, in rest_handle_action
    return rest_pull_close(body, restapi_repo, sdk_pr_target_repo, sdkbase, sdk_tag)
  File "/usr/local/lib/python3.6/dist-packages/swaggertosdk/restapi/github.py", line 185, in rest_pull_close
    rest_pr_management(rest_pr, sdk_pr_target_repo, sdk_tag, sdk_default_base)
  File "/usr/local/lib/python3.6/dist-packages/swaggertosdk/restapi/github_handler.py", line 151, in rest_pr_management
    sdk_tag=sdk_tag
  File "/usr/local/lib/python3.6/dist-packages/swaggertosdk/SwaggerToSdkNewCLI.py", line 254, in generate_sdk_from_git_object
    with manage_git_folder(gh_token, Path(temp_dir) / Path("rest"), branched_rest_api_id, pr_number=pr_number) as restapi_git_folder, \
  File "/usr/lib/python3.6/contextlib.py", line 81, in __enter__
    return next(self.gen)
  File "/usr/local/lib/python3.6/dist-packages/azure_devtools/ci_tools/github_tools.py", line 272, in manage_git_folder
    clone_to_path(gh_token, temp_dir, split_git_id[0], branch_or_commit=branch, pr_number=pr_number)
  File "/usr/local/lib/python3.6/dist-packages/azure_devtools/ci_tools/github_tools.py", line 212, in clone_to_path
    repo.git.checkout(branch_or_commit)
  File "/usr/local/lib/python3.6/dist-packages/git/cmd.py", line 548, in <lambda>
    return lambda *args, **kwargs: self._call_process(name, *args, **kwargs)
  File "/usr/local/lib/python3.6/dist-packages/git/cmd.py", line 1014, in _call_process
    return self.execute(call, **exec_kwargs)
  File "/usr/local/lib/python3.6/dist-packages/git/cmd.py", line 825, in execute
    raise GitCommandError(command, status, stderr_value, stdout_value)
git.exc.GitCommandError: Cmd('git') failed due to: exit code(128)
  cmdline: git checkout 9523e550cd50fd385b27497d81d904c994a1ff98
  stderr: 'fatal: reference is not a tree: 9523e550cd50fd385b27497d81d904c994a1ff98'

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

adxsdknet added a commit to adxsdknet/azure-sdk-for-net that referenced this pull request May 8, 2019
REST Spec PR 'Azure/azure-rest-api-specs#5889'
REST Spec PR Author 'dandanwang0320'
REST Spec PR Last commit
@adxsdknet
Copy link

Automation for azure-sdk-for-net

A PR has been created for you:
Azure/azure-sdk-for-net#6185
.NET SDK Commits:
adxsdknet/azure-sdk-for-net@a5e6437

@jhendrixMSFT jhendrixMSFT added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label May 8, 2019
Copy link
Member

@jhendrixMSFT jhendrixMSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new swagger isn't referenced in any README.md file so it won't get included in any SDK. Is this expected?

@AutorestCI
Copy link

AutorestCI commented May 8, 2019

Automation for azure-sdk-for-java

A PR has been created for you based on this PR content.

Once this PR will be merged, content will be added to your service PR:
Azure/azure-sdk-for-java#3337

@dandanwang0320
Copy link
Contributor Author

dandanwang0320 commented May 8, 2019

The new swagger isn't referenced in any README.md file so it won't get included in any SDK. Is this expected?

I added the readme files. In the readme.md file, ## Suppression part, I didn't add any of my new properties there. Should I?

@jhendrixMSFT
Copy link
Member

Suppressions are added only if there's a linter error that can't be fixed for some legitimate reason, so no need to add anything there at present.

"$ref": "#/definitions/JitRequestDefinitionListResult"
}
},
"404": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This indicates that a 404 should be treated as a success, is that intentional? ARM feedback will be interesting here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was following the convention of previous versions and other ARM APIs. Is there a general guideline for 404?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've not seen this before so it was a surprise to me, doesn't necessarily mean it's incorrect. The ARM reviewer can provide a definitive answer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know who's the ARM reviewer? If it's from ARM team, I could talk to them offline.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like one hasn't been assigned yet. If this is urgent you can email the ARM on-call person (there are instructions on how to find this info in the ARM API Review Checklist comment at the top.

@jhendrixMSFT
Copy link
Member

There are model validation failures that need to be fixed, please see the log here.

@dandanwang0320
Copy link
Contributor Author

There are model validation failures that need to be fixed, please see the log here.

Actually none of those warnings were add my new API or schema. Or is there anything needs to be added for existing apis in new version?

@jhendrixMSFT
Copy link
Member

They will need to be cleaned up at some point, if you don't want to clean them up now (e.g. tight deadline) we can skip it however can you please open an issue to track fixing them?

Copy link
Contributor

@KrisBash KrisBash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see comments for requested changes

@@ -0,0 +1,1885 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the future, please copy the last reviewed API version into the new version folder as the first commit. This enables an expedited review as we can diff the changes between commits.

],
"description": "The managed application provider authorization."
},
"ErrorResponse": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually comes with original schema, not changed by this PR.

}
}
},
"put": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is modeled as a tracked resource. Thus, it requires a PATCH api that supports update of "tags", but would ideally support update of all mutable properties

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have a patch api for this resource in our code base. Do you mean we should add that to our back end code?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes - portal/cli will not be able to update tags on the resource without it

@jhendrixMSFT
Copy link
Member

@dandanwang0320 hello! Any update here? Also there's a merge conflict that needs to be resolved.

@ravbhatnagar
Copy link
Contributor

@dandanwang0320 - please let me know when this is ready for re-review.

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Jul 2, 2019

In Testing, Please Ignore

[Logs] (Generated from e9b6dd7, Iteration 7)

Failed Python: test-repo-billy/azure-sdk-for-python [Logs]
  • No packages generated.
In-Progress Java: test-repo-billy/azure-sdk-for-java [Logs]
  • Package generation in progress.
In-Progress Go: test-repo-billy/azure-sdk-for-go [Logs] [Diff]
  • In-Progress preview/resources/mgmt/2015-10-01-preview [Logs]
  • In-Progress preview/resources/mgmt/2016-09-01-preview [Logs]
  • In-Progress preview/resources/mgmt/2017-06-01-preview [Logs]
  • In-Progress resources/mgmt/2015-01-01 [Logs]
  • In-Progress resources/mgmt/2015-11-01 [Logs]
  • In-Progress resources/mgmt/2015-12-01 [Logs]
  • In-Progress resources/mgmt/2016-02-01 [Logs]
  • In-Progress resources/mgmt/2016-04-01 [Logs]
  • In-Progress resources/mgmt/2016-06-01 [Logs]
  • In-Progress resources/mgmt/2016-07-01 [Logs]
  • In-Progress resources/mgmt/2016-09-01 [Logs]
  • In-Progress resources/mgmt/2016-12-01 [Logs]
  • In-Progress resources/mgmt/2017-05-10 [Logs]
  • In-Progress resources/mgmt/2017-09-01 [Logs]
  • In-Progress resources/mgmt/2018-02-01 [Logs]
  • In-Progress resources/mgmt/2018-03-01 [Logs]
  • In-Progress resources/mgmt/2018-05-01 [Logs]
  • In-Progress resources/mgmt/2018-06-01 [Logs]
  • In-Progress resources/mgmt/2019-01-01 [Logs]
  • In-Progress resources/mgmt/2019-03-01 [Logs]
  • In-Progress resources/mgmt/2019-05-01 [Logs]
Warning JavaScript: test-repo-billy/azure-sdk-for-js [Logs] [Diff]
  • Warning @azure/arm-features [Logs]
  • Warning @azure/arm-links [Logs]
  • Warning @azure/arm-locks [Logs]
  • Warning @azure/arm-managedapplications [Logs]
  • Warning @azure/arm-policy [Logs]
  • Warning @azure/arm-resources [Logs]

@dandanwang0320
Copy link
Contributor Author

@KrisBash Hi, would you have time to take another look?

Merge branch 'master' into AddJitRequest

# Conflicts:
#	specification/resources/resource-manager/readme.md
@PhoenixHe-NV
Copy link

@OpenAPIBot sdkautomation rebuild

@jhendrixMSFT
Copy link
Member

@dandanwang0320 hello is this ready for review by ARM?

@yungezz
Copy link
Member

yungezz commented Aug 15, 2019

@dandanwang0320 can we close the PR for now since you're not ready for review, and the PR is over several months. Pls create new PR when you're ready for review and merge.

@jhendrixMSFT
Copy link
Member

Closing due to inactivity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants