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

config: allow "latest" pseudo-version for Azure TDX config values #3166

Merged
merged 6 commits into from
Jun 27, 2024
Merged

config: allow "latest" pseudo-version for Azure TDX config values #3166

merged 6 commits into from
Jun 27, 2024

Conversation

daniel-weisse
Copy link
Member

@daniel-weisse daniel-weisse commented Jun 13, 2024 •
edited
Loading

Context

Attestation values in the config for Azure TDX currently use default values that require manual updates by users if newer versions are published.
Instead, TDX attestation variants should offer the same usability feature as SEV-SNP variants, where these values can be set to "latest", which causes them to be fetched and replaced by the latest recorded attestation values from our API.

Proposed change(s)

  • Enable Azure TDX config values to use "latest" as placeholder
  • Refactor the code in /internal/api/attestationconfigapi/ to allow for more attestation variants than just SEV-SNP

Additional info

Checklist

@netlify Netlify
Copy link

netlify bot commented Jun 13, 2024 •
edited
Loading

✅ Deploy Preview for constellation-docs canceled.

Name Link
🔨 Latest commit 03332ba
🔍 Latest deploy log https://app.netlify.com/sites/constellation-docs/deploys/667d09a9a38953000911ff52

@daniel-weisse daniel-weisse added the feature This introduces new functionality label Jun 13, 2024
@daniel-weisse daniel-weisse force-pushed the feat/api/tdx-versions branch 13 times, most recently from 3b1694b to d84cafe Compare June 17, 2024 13:11
@daniel-weisse daniel-weisse marked this pull request as ready for review June 17, 2024 13:34
@daniel-weisse daniel-weisse added this to the v2.17.0 milestone Jun 18, 2024
msanft
msanft approved these changes Jun 18, 2024
View reviewed changes
Copy link
Contributor

@msanft msanft left a comment

Choose a reason for hiding this comment

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

Didn't look at all of the refactorings, but the TDX support and CI changes LGTM.

@derpsteb derpsteb removed their request for review June 18, 2024 08:49
burgerdev
burgerdev reviewed Jun 21, 2024
View reviewed changes
Copy link
Contributor

@burgerdev burgerdev left a comment

Choose a reason for hiding this comment

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

Although I'm only halfway through, I wanted to leave a few comments.

internal/api/attestationconfigapi/cli/client/client.go Outdated Show resolved Hide resolved
internal/api/attestationconfigapi/cli/client/reporter.go Show resolved Hide resolved
internal/api/attestationconfigapi/cli/client/reporter.go Show resolved Hide resolved
internal/api/attestationconfigapi/cli/client/reporter.go Show resolved Hide resolved
internal/api/attestationconfigapi/cli/client/reporter.go Show resolved Hide resolved
internal/api/attestationconfigapi/cli/client/reporter.go Show resolved Hide resolved
internal/api/attestationconfigapi/cli/client/reporter.go Outdated Show resolved Hide resolved
elchead
elchead approved these changes Jun 25, 2024
View reviewed changes
Copy link
Contributor

@elchead elchead left a comment

Choose a reason for hiding this comment

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

I looked at the attestationconfigapi, upload tool and the TF provider changes.

@daniel-weisse daniel-weisse force-pushed the feat/api/tdx-versions branch 2 times, most recently from a8623d3 to 0926c96 Compare June 26, 2024 13:41
@github-actions GitHub Actions
Copy link
Contributor

Coverage report

Package Old New Trend
cli/internal/cmd 41.00% 41.00% 🚧
internal/api/attestationconfigapi 24.00% 41.70% ↗️
internal/api/attestationconfigapi/cli 0.00% 0.00% 🚧
internal/api/attestationconfigapi/cli/client 0.00% 7.10% 🆕
internal/api/client 0.00% 0.00% 🚧
internal/api/fetcher 0.00% 0.00% 🚧
internal/api/versionsapi 53.70% 54.50% ↗️
internal/attestation/azure/tdx 8.80% 8.70% ↘️
internal/config 68.00% 67.00% ↘️
internal/config/migration 0.00% 0.00% 🚧
terraform-provider-constellation/internal/provider 3.60% 3.60% 🚧

@github-actions GitHub Actions
Copy link
Contributor

Coverage report

Package Old New Trend
cli/internal/cmd 41.00% 41.00% 🚧
internal/api/attestationconfigapi 24.00% 41.70% ↗️
internal/api/attestationconfigapi/cli 0.00% 0.00% 🚧
internal/api/attestationconfigapi/cli/client 0.00% 7.10% 🆕
internal/api/client 0.00% 0.00% 🚧
internal/api/fetcher 0.00% 0.00% 🚧
internal/api/versionsapi 53.70% 54.50% ↗️
internal/attestation/azure/tdx 8.80% 8.70% ↘️
internal/config 68.00% 67.00% ↘️
internal/config/migration 0.00% 0.00% 🚧
terraform-provider-constellation/internal/provider 3.60% 3.60% 🚧

@daniel-weisse daniel-weisse merged commit d67d0ac into main Jun 27, 2024
9 checks passed
@daniel-weisse daniel-weisse deleted the feat/api/tdx-versions branch June 27, 2024 07:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@burgerdev burgerdev burgerdev approved these changes

@derpsteb derpsteb derpsteb approved these changes

@elchead elchead elchead approved these changes

@msanft msanft msanft approved these changes

Assignees
No one assigned
Labels
feature This introduces new functionality
Projects
None yet
Milestone
v2.17.0
Development

Successfully merging this pull request may close these issues.
5 participants
@daniel-weisse @burgerdev @derpsteb @elchead @msanft