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.

Sign up for GitHub

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

Jump to bottom

Added a step to validate rest api spec location before release #40783

Merged
merged 14 commits into from
Jan 16, 2024
Merged

Added a step to validate rest api spec location before release #40783

merged 14 commits into from
Jan 16, 2024

Conversation

raych1
Copy link
Member

@raych1 raych1 commented Dec 14, 2023

The purpose is to gate on the REST API specifications used to release SDK are from the main branch of Azure/azure-rest-api-specs repository.

Added a step for the validation to the release pipeline.

The common script and pipeline are in another PR.
Azure/azure-sdk-tools#7451

Holding off this PR merge until the above PR is merged and synced to dotnet repository.

@weshaggard , can you review?

@raych1 raych1 self-assigned this Dec 14, 2023
jsquire
jsquire previously requested changes Dec 14, 2023
View reviewed changes
Copy link
Member

@jsquire jsquire left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I'd like to understand the impact of this for libraries that are not generated. In looking over the script, a warning is generated in the case that neither an autorest nor typespec reference file is found, despite that being a legitimate scenario.

@raych1
Copy link
Member Author

raych1 commented Dec 15, 2023

I'd like to understand the impact of this for libraries that are not generated. In looking over the script, a warning is generated in the case that neither an autorest nor typespec reference file is found, despite that being a legitimate scenario.

It would log a warning if the code hasn't been generated but doesn't fail the pipeline. It doesn't impact followed steps in the pipeline.

@raych1 raych1 requested review from chunyu3 and lirenhe December 18, 2023 02:33
@raych1
Copy link
Member Author

raych1 commented Jan 11, 2024 •
edited
Loading

@jsquire , I've modified the common script to halt execution silently if the configuration value is absent. This validation will only occur during a GA package release, and only if the configuration value is valid.

@jsquire jsquire dismissed their stale review January 11, 2024 16:25

Blocking issue has been addressed.

@jsquire
Copy link
Member

jsquire commented Jan 11, 2024

@jsquire , I've modified the common script to halt execution silently if the configuration value is absent. This validation will only occur during a GA package release, and only if the configuration value is valid.

Thanks. I've removed my block.

weshaggard
weshaggard approved these changes Jan 12, 2024
View reviewed changes
Copy link
Member

@weshaggard weshaggard left a comment

Choose a reason for hiding this comment

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

Looks reasonable. Please be sure to test this with the template release.

@raych1
Copy link
Member Author

raych1 commented Jan 15, 2024 •
edited
Loading

This is the test pipeline result for template service, and the result looks as expected.

@weshaggard
Copy link
Member

Looks like the test run is https://dev.azure.com/azure-sdk/internal/_build/results?buildId=3406027&view=logs&j=7bb69cc7-6f52-585e-8b13-5bae7c4a9fa4&t=7abbbe6b-d005-5c95-feb6-99ffe4e699e1.

Have we tested it with a GA library? If we cannot easily test this we need watch this check closely over the next set of releases to ensure we don't hit any blocking bugs.

@raych1 raych1 requested a review from heaths as a code owner January 16, 2024 01:16
@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

@raych1
Copy link
Member Author

raych1 commented Jan 16, 2024

Looks like the test run is https://dev.azure.com/azure-sdk/internal/_build/results?buildId=3406027&view=logs&j=7bb69cc7-6f52-585e-8b13-5bae7c4a9fa4&t=7abbbe6b-d005-5c95-feb6-99ffe4e699e1.

Have we tested it with a GA library? If we cannot easily test this we need watch this check closely over the next set of releases to ensure we don't hit any blocking bugs.

@weshaggard , I modified the version to 1.0.3 in ".csproj" file but the version was updated to 'beta' again by the "setTestPipelineVersion" script in pipeline. Anyway, I pushed some temp changes to test a GA library and the followed spec location validation. As the autorest.md of template project uses local swagger path, the validation cannot pass. Changing it to the remote path would fail the analyze step. But I validated the step of search commit in the script by temp changes too, the github api call worked fine. This is the test run and result is as:

ServiceDir:template, PackageName:Azure.Template. Commit 05c4049bc22f0ec65acc18f9835132397049cb9e exists in 'main' branch of Azure/azure-rest-api-specs repository.
[debug]ServiceDir:template, PackageName:Azure.Template. 'input-file' is set as:$(this-folder)/swagger/mini-secrets.json
##[error]ServiceDir:template, PackageName:Azure.Template. Invalid file url: $(this-folder)/swagger/mini-secrets.json
##[error]PowerShell exited with code '1'.

I think this PR is ready to merge.

@weshaggard
Copy link
Member

Ok let's be on the lookout for any fallout of this change as releases progress.

@raych1 raych1 merged commit 8059b65 into main Jan 16, 2024
63 checks passed
@raych1 raych1 deleted the user/raych1/pipeline-verify-spec-location branch January 16, 2024 23:49
This was referenced Jan 17, 2024
Merged
Merged
Merged
jeremymeng pushed a commit to Azure/azure-sdk-for-js that referenced this pull request Jan 19, 2024
@raych1
Added pipeline to validate spec location (#28303)
c719a2c 
### Purpose of this PR
The purpose is to gate on the REST API specifications used to release
SDK are from the main branch of
[Azure/azure-rest-api-spec](https://github.com/Azure/azure-rest-api-specs).

### Packages impacted by this PR
All the data plane GA packages to be released.

I scanned through this JS repo and identified 11 data plane services
that have released GA packages but their configuration file either
`tsp-location.yaml` or `swagger\README.md` reference the REST API specs
from a non-main branch of Azure spec repo.
Please refer to [this
spreadsheet](https://microsoftapc-my.sharepoint.com/:x:/g/personal/raychen_microsoft_com/ETMHnmWSA6VKmGouTjAITwEBqSavTLQkOcd2Ykx2eEE8_g?e=oalb1N)
to get the details of all the violated packages.

After this PR is merged, the next GA release for these packages will be
blocked. They need to correct the spec path before releasing GA
packages.

### Describe the problem that is addressed by this PR
Added a pipeline from common template to release pipeline to validate
the REST API specification location is from the main branch of
[Azure/azure-rest-api-spec](https://github.com/Azure/azure-rest-api-specs).

This validation will be skipped if the package to be released is non-GA
package.

### Provide a list of related PRs
The common script and pipeline were merged by another PR.
Azure/azure-sdk-tools#7451

FYI - The same change has been added to other language repos' release
pipelines.
Azure/azure-sdk-for-net#40783
Azure/azure-sdk-for-java#38332
@raych1 raych1 mentioned this pull request Jan 26, 2024
Merged
@jsquire jsquire mentioned this pull request Apr 3, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jsquire jsquire jsquire left review comments

@weshaggard weshaggard weshaggard approved these changes

@hallipr hallipr Awaiting requested review from hallipr hallipr is a code owner

@benbp benbp Awaiting requested review from benbp benbp is a code owner

@chunyu3 chunyu3 Awaiting requested review from chunyu3

@lirenhe lirenhe Awaiting requested review from lirenhe

@heaths heaths Awaiting requested review from heaths

Assignees

@raych1 raych1

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@raych1 @jsquire @weshaggard @azure-sdk