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

azurerm_attestation_provider - add AzureVM and SEV-SNP attestation types #22229

Merged
merged 4 commits into from
Jul 3, 2023

Conversation

msanft
Copy link
Contributor

@msanft msanft commented Jun 21, 2023

Context

Proposed Changes

  • Add the sev_snp_policy_base64 and azure_vm_policy_base64 fields to the azurerm_attestation_provider resource.
  • TODO: For now, I edited the enums.go file in the SDK manually to test the changes. I assume I would need to get a PR to the SDK upstreamed to get these changes in?

Related Issue

@msanft msanft changed the title Add AzureVM and SEV-SNP attestation types azurerm_attestation_provider - add AzureVM and SEV-SNP attestation types Jun 21, 2023
@stephybun
Copy link
Member

Thanks for this PR @msanft.

As you've already identified, we cannot accept changes to vendored files and an upstream change would need to happen before this could be merged.

The SDK used here is generated from the Azure REST API spec published in this repo. Unfortunately the latest dataplane version for attestation is still missing this value so you will need to raise an issue over on that repo to have it added to the spec before we can regenerate the SDK and propagate those changes into the provider to expose this value.

Since this isn't something we can currently support I'm going to close this issue for the time being, but once this value has been added to the spec we can look into reopening this and adding it.

Thanks!

@stephybun stephybun closed this Jun 28, 2023
@stephybun stephybun reopened this Jun 28, 2023
Copy link
Member

@stephybun stephybun left a comment

Choose a reason for hiding this comment

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

Apologies just saw that one of the enums is available - if you can remove the one that isn't available in the SDK from this PR then we can take look over this and get it merged

@msanft
Copy link
Contributor Author

msanft commented Jun 29, 2023

Apologies just saw that one of the enums is available - if you can remove the one that isn't available in the SDK from this PR then we can take look over this and get it merged

I'll edit that so we can have that merged. Thanks a lot!

For the AzureVM type, I will try to get that incorporated into the spec here and open a separate PR once it's available upstream.

@github-actions github-actions bot added size/S and removed size/M labels Jun 29, 2023
@msanft msanft requested a review from stephybun June 30, 2023 07:13
Copy link
Member

@stephybun stephybun left a comment

Choose a reason for hiding this comment

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

Thanks @msanft.

The change made to the vendored SDK needs to be reverted. We also have a test failure

------- Stdout: -------
=== RUN   TestAccAttestationProvider_withPolicy
=== PAUSE TestAccAttestationProvider_withPolicy
=== CONT  TestAccAttestationProvider_withPolicy
    testcase.go:113: Step 1/2 error: Error running apply: exit status 1
        Error: updating value for `tpm_policy_base64`: attestation.PolicyClient#Set: Failure responding to request: StatusCode=400 -- Original Error: autorest/azure: Service returned an error. Status=400 Code="Bad request" Message="Tpm attestation is not supported in the 'UKSouth' region."
          with azurerm_attestation_provider.test,
          on terraform_plugin_test.tf line 24, in resource "azurerm_attestation_provider" "test":
          24: resource "azurerm_attestation_provider" "test" {
--- FAIL: TestAccAttestationProvider_withPolicy (514.40s)
FAIL

@msanft
Copy link
Contributor Author

msanft commented Jun 30, 2023

Thanks @msanft.

The change made to the vendored SDK needs to be reverted. We also have a test failure

------- Stdout: -------
=== RUN   TestAccAttestationProvider_withPolicy
=== PAUSE TestAccAttestationProvider_withPolicy
=== CONT  TestAccAttestationProvider_withPolicy
    testcase.go:113: Step 1/2 error: Error running apply: exit status 1
        Error: updating value for `tpm_policy_base64`: attestation.PolicyClient#Set: Failure responding to request: StatusCode=400 -- Original Error: autorest/azure: Service returned an error. Status=400 Code="Bad request" Message="Tpm attestation is not supported in the 'UKSouth' region."
          with azurerm_attestation_provider.test,
          on terraform_plugin_test.tf line 24, in resource "azurerm_attestation_provider" "test":
          24: resource "azurerm_attestation_provider" "test" {
--- FAIL: TestAccAttestationProvider_withPolicy (514.40s)
FAIL

Hey @stephybun ,
Sorry for my negligence here, I reverted the changes to the vendored SDK.

re the failed test; Unfortunately, I cannot replicate the failure locally. Judging from the error message, I think we could either switch the region used to e.g. westus, which should have all attestation types, or simply not test the unavailable attestation types in uksouth. I think testing all types would of course be the preferred way, but if you're constrained with using UK-South, I can also remove the unavailable attestation types here.

Copy link
Member

@stephybun stephybun left a comment

Choose a reason for hiding this comment

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

I hope you don't mind I pushed the change to the location for the tests as well as formatted the test config to fix the CI - this now looks good to go. Thanks @msanft LGTM 💯

@stephybun stephybun merged commit edd4c2b into hashicorp:main Jul 3, 2023
@github-actions github-actions bot added this to the v3.64.0 milestone Jul 3, 2023
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for trusted launch and SEV-SNP attestation policy management
2 participants