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

[azure-security-attestation-cpp] Update to 1.1.0 #29516

Merged
merged 1 commit into from
Feb 9, 2023

Conversation

azure-sdk
Copy link
Contributor

Update vcpkg ports for Azure SDK release. This release may contain multiple ports.

## 1.1.0 (2023-02-07)

### Breaking Changes

- Changed `AttestationClient::AttestTpm` to match `AttestOpenEnclave` and `AttestSgxEnclave`:
  - Added `std::vector<uint8_t>` dataToAttest parameter to the `AttestTpm()` client method.
  - Removed `Payload` in `TpmAttestationOptions`.
  - Changed `TpmResult` in `TpmAttestationResult` to type `std::vector<uint8_t>`.
@azure-sdk
Copy link
Contributor Author

Adding azure-security-attestation to release

cc: @LarryOsterman, @gkostal, @anilba06, @kroshkina-ms, @ahmadmsft, @RickWinter, @ahsonkhan, @antkmsft, @vhvb1989, @gearama

@Cheney-W Cheney-W added the category:port-update The issue is with a library, which is requesting update new revision label Feb 8, 2023
@@ -1,8 +1,8 @@
vcpkg_from_github(
OUT_SOURCE_PATH SOURCE_PATH
REPO Azure/azure-sdk-for-cpp
REF azure-security-attestation_1.0.0
SHA512 75b616ea152a88b2cd3be261df134523f1743343542f005e85f1dc9d438584038d0d92879c3caabfd3bd60c6c37c658ad327b2fb62693d99c170e64182f6831f
REF azure-security-attestation_1.1.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
REF azure-security-attestation_1.1.0
REF "azure-security-attestation_${VERSION}"

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
Member

@antkmsft antkmsft Feb 8, 2023

Choose a reason for hiding this comment

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

Thank you @Cheney-W, we'll consider switching to use that feature in the future. I don't think we should take if for this PR, but please comment if you strongly believe that we should.

At this point, updating it for the port would require us to re-run the x-add-version command, but for the change to be permanent, in needs to go to our repo. We use file templates to produce manifest files for the vcpkg, and our CI does the version substitution. To fully update it, we'll need to update CI scripts as well. We could do it, but in the future.

More off topic:
There is one effort that I would like to happen, once we accumulate the set of changes that we need to apply.
I'd love to update our SDK so that we can use the same manifest files for development mode and for pushing the release to vcpkg. But currently, I think, it requires the version to be present in the vcpkg.json file, which won't work for us, because our source of truth for the version is the header file in the repo, from where it is taken and version replacement happens for the vcpkg manifest files during the port update. Which means, we can't as easily keep version-semver in the vcpkg.json to be in sync with the header file in our repo.

For this specific feature, I am not sure if it is necessarily convenient for development and for human error prevention. When you have aa specific tag there, you can be more certain that the vcpkg is trying to fetch the version that you expect and see. Besides, SHA512 needs to be updated anyways.
But, we need to think about it. I don't think we'd take it for this PR, unless it is a thing that you are applying across multiple ports as sort of the centralized effort. We surely want to be cooperative if that's the case.

BTW, is there a document where col features like this one with ${VERSION} are documented? Or maybe some "what's new" type of list with the newest features added that we can follow? We created the infrastructure for vcpkg releases about 2 years ago, and since then many good features were added. We did adopt some (switching from CONTROL file to vcpkg.json, using version>= once it went mainstream etc), but maybe there are even more that we can adopt.

Copy link
Contributor

Choose a reason for hiding this comment

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

we'll consider switching to use that feature in the future. I don't think we should take if for this PR

+1

Our releases aren't done manually.

We'd want to get this port update released and available for customers (as-is), and modify our repo and automation to use such new features, in the future.

BTW, is there a document where col features like this one with ${VERSION} are documented?

@BillyONeal, @ras0219-msft - thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

@Cheney-W Cheney-W added requires:author-response info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Feb 8, 2023
@JavierMatosD JavierMatosD merged commit e041234 into microsoft:master Feb 9, 2023
@ahsonkhan ahsonkhan deleted the azsdkcpp202302attestation branch February 9, 2023 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-update The issue is with a library, which is requesting update new revision info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants