Skip to content

Remove Azure.CodeSigning service from specifications#26635

Merged
tjprescott merged 1 commit intoAzure:mainfrom
Jaxelr:jaxel/remove-codesigning-service
Dec 5, 2023
Merged

Remove Azure.CodeSigning service from specifications#26635
tjprescott merged 1 commit intoAzure:mainfrom
Jaxelr:jaxel/remove-codesigning-service

Conversation

@Jaxelr
Copy link
Member

@Jaxelr Jaxelr commented Nov 9, 2023

Data Plane API - Pull Request

This PR is accompanied by the following related PR. The purpose of this PR is to remove the Azure codesigning service from the specification listing for the following reasons:

  • Azure Code Signing Service has been renamed to Azure Developer Signing Service
  • This SDK hasn't been published to any platform, therefore rendering it meaningless and confusing to keep it.
  • This was also discussed with the SDK team to use an appropriate strategy on how to handle the name change.

cc: @ashutak84, @DawnWang17

API Info: The Basics

Most of the information about your service should be captured in the issue that serves as your API Spec engagement record.

  • Link to API Spec engagement record issue:

Is this review for (select one):

  • a private preview
  • a public preview
  • GA release

Change Scope

This section will help us focus on the specific parts of your API that are new or have been modified.
Please share a link to the design document for the new APIs, a link to the previous API Spec document (if applicable), and the root paths that have been updated.

  • Design Document:
  • Previous API Spec Doc:
  • Updated paths:

Viewing API changes

For convenient view of the API changes made by this PR, refer to the URLs provided in the table
in the Generated ApiView comment added to this PR. You can use ApiView to show API versions diff.

Suppressing failures

If one or multiple validation error/warning suppression(s) is detected in your PR, please follow the
Swagger-Suppression-Process
to get approval.

❔Got questions? Need additional info?? We are here to help!

Contact us!

The Azure API Review Board is dedicated to helping you create amazing APIs. You can read about our mission and learn more about our process on our wiki.

Click here for links to tools, specs, guidelines & other good stuff

Tooling

Guidelines & Specifications

Helpful Links

@Jaxelr Jaxelr requested a review from a team as a code owner November 9, 2023 01:51
@Jaxelr Jaxelr requested review from MushMal and vicancy and removed request for a team November 9, 2023 01:51
@openapi-pipeline-app
Copy link

openapi-pipeline-app bot commented Nov 9, 2023

Next Steps to Merge

✔️ All automated merging requirements have been met! Refer to step 4 in the PR workflow diagram (even if your PR is for data plane, not ARM).

@openapi-pipeline-app
Copy link

openapi-pipeline-app bot commented Nov 9, 2023

Swagger Validation Report

️❌BreakingChange: 9 Errors, 0 Warnings failed [Detail]
compared swaggers (via Oad v0.10.4)] new version base version
azure.codesigning.json 2023-06-15-preview(f30d0bc) 2023-06-15-preview(main)
Rule Message
1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/codesigningaccounts/{codeSigningAccountName}/certificateprofiles/{certificateProfileName}:sign' removed or restructured?
Old: Azure.CodeSigning/preview/2023-06-15-preview/azure.codesigning.json#L54:5
1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/codesigningaccounts/{codeSigningAccountName}/certificateprofiles/{certificateProfileName}/sign/{operationId}' removed or restructured?
Old: Azure.CodeSigning/preview/2023-06-15-preview/azure.codesigning.json#L144:5
1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/codesigningaccounts/{codeSigningAccountName}/certificateprofiles/{certificateProfileName}/sign/eku' removed or restructured?
Old: Azure.CodeSigning/preview/2023-06-15-preview/azure.codesigning.json#L217:5
1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/codesigningaccounts/{codeSigningAccountName}/certificateprofiles/{certificateProfileName}/sign/rootcert' removed or restructured?
Old: Azure.CodeSigning/preview/2023-06-15-preview/azure.codesigning.json#L271:5
1006 - RemovedDefinition The new version is missing a definition that was found in the old version. Was 'CodeSigningSubmissionOptions' removed or renamed?
New: Azure.CodeSigning/preview/2023-06-15-preview/azure.codesigning.json#L41:3
Old: Azure.CodeSigning/preview/2023-06-15-preview/azure.codesigning.json#L319:3
1006 - RemovedDefinition The new version is missing a definition that was found in the old version. Was 'SignatureAlgorithm' removed or renamed?
New: Azure.CodeSigning/preview/2023-06-15-preview/azure.codesigning.json#L41:3
Old: Azure.CodeSigning/preview/2023-06-15-preview/azure.codesigning.json#L319:3
1007 - RemovedClientParameter The new version is missing a client parameter that was found in the old version. Was 'Azure.Core.Foundations.ApiVersionParameter' removed or renamed?
New: Azure.CodeSigning/preview/2023-06-15-preview/azure.codesigning.json#L42:3
Old: Azure.CodeSigning/preview/2023-06-15-preview/azure.codesigning.json#L665:3
1007 - RemovedClientParameter The new version is missing a client parameter that was found in the old version. Was 'CodeSigningOptions.certificateProfileName' removed or renamed?
New: Azure.CodeSigning/preview/2023-06-15-preview/azure.codesigning.json#L42:3
Old: Azure.CodeSigning/preview/2023-06-15-preview/azure.codesigning.json#L665:3
1007 - RemovedClientParameter The new version is missing a client parameter that was found in the old version. Was 'CodeSigningOptions.codeSigningAccountName' removed or renamed?
New: Azure.CodeSigning/preview/2023-06-15-preview/azure.codesigning.json#L42:3
Old: Azure.CodeSigning/preview/2023-06-15-preview/azure.codesigning.json#L665:3
️️✔️Breaking Change(Cross-Version) succeeded [Detail] [Expand]
There are no breaking changes.
️️✔️CredScan succeeded [Detail] [Expand]
There is no credential detected.
️️✔️LintDiff succeeded [Detail] [Expand]
Validation passes for LintDiff.
compared tags (via openapi-validator v2.1.6) new version base version
default default(f30d0bc) default(main)
️❌Avocado: 3 Errors, 0 Warnings failed [Detail]
Rule Message
MISSING_README Can not find readme.md in the folder. If no readme.md file, it will block SDK generation.
folder: data-plane/Azure.CodeSigning/preview/2023-06-15-preview
MISSING_README Can not find readme.md in the folder. If no readme.md file, it will block SDK generation.
folder: Azure.CodeSigning/preview/2023-06-15-preview/examples
MISSING_README Can not find readme.md in the folder. If no readme.md file, it will block SDK generation.
folder: f30d0bcc7ebad084bd500e52648349398005566f/specification/codesigning/data-plane
️️✔️SwaggerAPIView succeeded [Detail] [Expand]
️️✔️TypeSpecAPIView succeeded [Detail] [Expand]
️️✔️ModelValidation succeeded [Detail] [Expand]
Validation passes for ModelValidation.
️️✔️SemanticValidation succeeded [Detail] [Expand]
Validation passes for SemanticValidation.
️️✔️PoliCheck succeeded [Detail] [Expand]
Validation passed for PoliCheck.
️️✔️SpellCheck succeeded [Detail] [Expand]
Validation passes for SpellCheck.
️️✔️Lint(RPaaS) succeeded [Detail] [Expand]
Validation passes for Lint(RPaaS).
️❌PR Summary: 0 Errors, 0 Warnings failed [Detail]
️️✔️Automated merging requirements met succeeded [Detail] [Expand]
Posted by Swagger Pipeline | How to fix these errors?

@openapi-pipeline-app
Copy link

openapi-pipeline-app bot commented Nov 9, 2023

Swagger Generation Artifacts

️️✔️ApiDocPreview succeeded [Detail] [Expand]
️️✔️SDK Breaking Change Tracking succeeded [Detail] [Expand]

Breaking Changes Tracking

Posted by Swagger Pipeline | How to fix these errors?

@openapi-pipeline-app
Copy link

openapi-pipeline-app bot commented Nov 9, 2023

PR validation pipeline restarted successfully. If there is ApiView generated, it will be updated in this comment.

@Jaxelr
Copy link
Member Author

Jaxelr commented Nov 21, 2023

hi @vicancy @MushMal any help approving this PR? For context, please see the linked PR as well. Thanks!

Copy link
Member

@vicancy vicancy left a comment

Choose a reason for hiding this comment

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

LGTM, I think it also need breaking change approval, adding @JeffreyRichter for help

@JeffreyRichter
Copy link
Member

You need to justify these breaks in an already-shipped api-version: https://github.com/Azure/azure-rest-api-specs/pull/26635/checks?check_run_id=18859492990

@Jaxelr
Copy link
Member Author

Jaxelr commented Nov 28, 2023

@JeffreyRichter even if the api was never published? In this case the api exists on this PR now.

@JeffreyRichter
Copy link
Member

I don't understand. This PR is for an api-version in the main branch of the public repo so it IS published.
Are you saying that this service version is not live yet?
If so, then why did this api-version get pushed to main and then broken in place?

@Jaxelr
Copy link
Member Author

Jaxelr commented Nov 29, 2023

@JeffreyRichter Apologies for the confusion, I understand it is published on the public repository, unfortunately after doing the merge, there was an internal branding discussion within the Azure Code Signing team and the result was that the service has been renamed to Azure Developer Signing, this PR does the addition of the same api/api version under the new name. Prior to submitting this PR this matter was also discussed with the Azure stewardship team and the recommended course of action was to segregate in 2 PRs the removal and addition of the same service under the new name.

@JeffreyRichter JeffreyRichter added the Approved-BreakingChange DO NOT USE! OBSOLETE label. See https://github.com/Azure/azure-sdk-tools/issues/6374 label Nov 29, 2023
Copy link
Member

@MushMal MushMal left a comment

Choose a reason for hiding this comment

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

Seems Jeffreys concern has been addressed.

@tjprescott tjprescott merged commit 3556bb9 into Azure:main Dec 5, 2023
@Jaxelr Jaxelr deleted the jaxel/remove-codesigning-service branch December 5, 2023 19:21
demoray added a commit to demoray/azure-sdk-for-rust that referenced this pull request Dec 8, 2023
As identified in Azure#1479, we do not currently handle deleted/renamed specifications well.

This update addresses this via the following:
* Moves to using parsing `cargo_toml` to parse services/Cargo.toml to know what crates already exist
* Replaces all uses of `list_crate_names` with using the results of `gen_crates`

As a byproduct, this identifies that the previously existing spec that would result in `azure_svc_codesigning` was removed.

ref: Azure/azure-rest-api-specs#26635
demoray added a commit to Azure/azure-sdk-for-rust that referenced this pull request Dec 8, 2023
As identified in #1479, we do not currently handle deleted/renamed specifications well.

This update addresses this via the following:
* Moves to using parsing `cargo_toml` to parse services/Cargo.toml to know what crates already exist
* Replaces all uses of `list_crate_names` with using the results of `gen_crates`

As a byproduct, this identifies that the previously existing spec that would result in `azure_svc_codesigning` was removed.

ref: Azure/azure-rest-api-specs#26635
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved-BreakingChange DO NOT USE! OBSOLETE label. See https://github.com/Azure/azure-sdk-tools/issues/6374

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants