From 6e6063a7166614b0657febb5d06157a7750f7208 Mon Sep 17 00:00:00 2001 From: Adrian Stobbe Date: Tue, 28 Nov 2023 09:43:02 +0100 Subject: [PATCH] pr feedback --- cli/internal/cmd/configfetchmeasurements.go | 3 +- .../cmd/configfetchmeasurements_test.go | 19 +++++---- .../measurements/measurements_test.go | 7 ---- .../docs/data-sources/attestation.md | 19 --------- .../constellation_attestation/data-source.tf | 19 --------- .../provider/attestation_data_source.go | 39 ++++++++++++------- .../internal/provider/provider.go | 2 +- .../internal/provider/provider_test.go | 4 -- 8 files changed, 39 insertions(+), 73 deletions(-) diff --git a/cli/internal/cmd/configfetchmeasurements.go b/cli/internal/cmd/configfetchmeasurements.go index 83993b5292..59a40140a4 100644 --- a/cli/internal/cmd/configfetchmeasurements.go +++ b/cli/internal/cmd/configfetchmeasurements.go @@ -112,8 +112,7 @@ func runConfigFetchMeasurements(cmd *cobra.Command, _ []string) error { } func (cfm *configFetchMeasurementsCmd) configFetchMeasurements( - cmd *cobra.Command, - fileHandler file.Handler, fetcher attestationconfigapi.Fetcher, + cmd *cobra.Command, fileHandler file.Handler, fetcher attestationconfigapi.Fetcher, ) error { if !cfm.canFetchMeasurements { cmd.PrintErrln("Fetching measurements is not supported in the OSS build of the Constellation CLI. Consult the documentation for instructions on where to download the enterprise version.") diff --git a/cli/internal/cmd/configfetchmeasurements_test.go b/cli/internal/cmd/configfetchmeasurements_test.go index d8c7bdaa26..591785118f 100644 --- a/cli/internal/cmd/configfetchmeasurements_test.go +++ b/cli/internal/cmd/configfetchmeasurements_test.go @@ -153,11 +153,16 @@ func newTestClient(fn roundTripFunc) *http.Client { func TestConfigFetchMeasurements(t *testing.T) { testCases := map[string]struct { insecureFlag bool + err error wantErr bool - isRekorErr bool }{ + "no error succeeds": {}, "failing rekor verify should not result in error": { - isRekorErr: true, + err: measurements.ErrRekor, + }, + "error other than Rekor fails": { + err: assert.AnError, + wantErr: true, }, } @@ -174,7 +179,8 @@ func TestConfigFetchMeasurements(t *testing.T) { err := fileHandler.WriteYAML(constants.ConfigFilename, gcpConfig, file.OptMkdirAll) require.NoError(err) - cfm := &configFetchMeasurementsCmd{canFetchMeasurements: true, log: logger.NewTest(t), verifyFetcher: stubVerifyFetcher{isRekorErr: tc.isRekorErr}} + fetcher := stubVerifyFetcher{err: tc.err} + cfm := &configFetchMeasurementsCmd{canFetchMeasurements: true, log: logger.NewTest(t), verifyFetcher: fetcher} cfm.flags.insecure = tc.insecureFlag cfm.flags.force = true @@ -189,14 +195,11 @@ func TestConfigFetchMeasurements(t *testing.T) { } type stubVerifyFetcher struct { - isRekorErr bool + err error } func (f stubVerifyFetcher) FetchAndVerifyMeasurements(_ context.Context, _ string, _ cloudprovider.Provider, _ variant.Variant, _ bool) (measurements.M, error) { - if f.isRekorErr { - return nil, measurements.ErrRekor - } - return nil, nil + return nil, f.err } type stubAttestationFetcher struct{} diff --git a/internal/attestation/measurements/measurements_test.go b/internal/attestation/measurements/measurements_test.go index b8aeef7e2c..73cee74795 100644 --- a/internal/attestation/measurements/measurements_test.go +++ b/internal/attestation/measurements/measurements_test.go @@ -10,8 +10,6 @@ import ( "bytes" "context" "encoding/json" - "errors" - "fmt" "io" "net/http" "net/url" @@ -1045,8 +1043,3 @@ func TestMeasurementsCompare(t *testing.T) { }) } } - -func TestRekorErrCheck(t *testing.T) { - err := fmt.Errorf("%w: %w", ErrRekor, errors.New("test")) - assert.ErrorIs(t, err, ErrRekor) -} diff --git a/terraform-provider-constellation/docs/data-sources/attestation.md b/terraform-provider-constellation/docs/data-sources/attestation.md index 1887100471..16f38b5e27 100644 --- a/terraform-provider-constellation/docs/data-sources/attestation.md +++ b/terraform-provider-constellation/docs/data-sources/attestation.md @@ -13,30 +13,11 @@ The data source to fetch measurements from a configured cloud provider and image ## Example Usage ```terraform -terraform { - required_providers { - constellation = { - source = "registry.terraform.io/edgelesssys/constellation" - } - } -} - -provider "constellation" { -} - data "constellation_attestation" "test" { csp = "aws" attestation_variant = "aws-sev-snp" image_version = "v2.13.0" } - -output "measurements" { - value = data.constellation_attestation.test.measurements -} - -output "attestation" { - value = data.constellation_attestation.test.attestation -} ``` diff --git a/terraform-provider-constellation/examples/data-sources/constellation_attestation/data-source.tf b/terraform-provider-constellation/examples/data-sources/constellation_attestation/data-source.tf index 46241d0ad9..4b40c0a69c 100644 --- a/terraform-provider-constellation/examples/data-sources/constellation_attestation/data-source.tf +++ b/terraform-provider-constellation/examples/data-sources/constellation_attestation/data-source.tf @@ -1,24 +1,5 @@ -terraform { - required_providers { - constellation = { - source = "registry.terraform.io/edgelesssys/constellation" - } - } -} - -provider "constellation" { -} - data "constellation_attestation" "test" { csp = "aws" attestation_variant = "aws-sev-snp" image_version = "v2.13.0" } - -output "measurements" { - value = data.constellation_attestation.test.measurements -} - -output "attestation" { - value = data.constellation_attestation.test.attestation -} diff --git a/terraform-provider-constellation/internal/provider/attestation_data_source.go b/terraform-provider-constellation/internal/provider/attestation_data_source.go index 3a6230afad..bfc7e61dc7 100644 --- a/terraform-provider-constellation/internal/provider/attestation_data_source.go +++ b/terraform-provider-constellation/internal/provider/attestation_data_source.go @@ -178,13 +178,20 @@ func (d *AttestationDataSource) Read(ctx context.Context, req datasource.ReadReq csp := cloudprovider.FromString(data.CSP.ValueString()) if csp == cloudprovider.Unknown { - resp.Diagnostics.AddError("Unknown CSP", fmt.Sprintf("Unknown CSP: %s", data.CSP.ValueString())) + resp.Diagnostics.AddAttributeError( + path.Root("csp"), + "Invalid CSP", + fmt.Sprintf("Invalid CSP: %s", data.CSP.ValueString()), + ) return } attestationVariant, err := variant.FromString(data.AttestationVariant.ValueString()) if err != nil { - resp.Diagnostics.AddError("Unknown Attestation Variant", - fmt.Sprintf("Unknown Attestation Variant: %s", data.AttestationVariant.ValueString())) + resp.Diagnostics.AddAttributeError( + path.Root("attestation_variant"), + "Invalid Attestation Variant", + fmt.Sprintf("Invalid attestation variant: %s", data.CSP.ValueString()), + ) return } if attestationVariant.Equal(variant.AzureSEVSNP{}) || attestationVariant.Equal(variant.AWSSEVSNP{}) { @@ -193,8 +200,11 @@ func (d *AttestationDataSource) Read(ctx context.Context, req datasource.ReadReq resp.Diagnostics.AddError("Fetching SNP Version numbers", err.Error()) return } - tfSnpVersions := convertSNPAttestationTfStateCompatible(resp, attestationVariant, snpVersions) - diags := resp.State.SetAttribute(ctx, path.Root("attestation"), tfSnpVersions) + tfSnpAttestation, err := convertSNPAttestationTfStateCompatible(attestationVariant, snpVersions) + if err != nil { + resp.Diagnostics.AddError("Converting SNP attestation", err.Error()) + } + diags := resp.State.SetAttribute(ctx, path.Root("attestation"), tfSnpAttestation) resp.Diagnostics.Append(diags...) if resp.Diagnostics.HasError() { return @@ -221,9 +231,9 @@ func (d *AttestationDataSource) Read(ctx context.Context, req datasource.ReadReq tflog.Trace(ctx, "read constellation attestation data source") } -func convertSNPAttestationTfStateCompatible(resp *datasource.ReadResponse, attestationVariant variant.Variant, +func convertSNPAttestationTfStateCompatible(attestationVariant variant.Variant, snpVersions attestationconfigapi.SEVSNPVersionAPI, -) sevSnpAttestation { +) (tfSnpAttestation sevSnpAttestation, err error) { var cert config.Certificate switch attestationVariant.(type) { case variant.AWSSEVSNP: @@ -233,9 +243,9 @@ func convertSNPAttestationTfStateCompatible(resp *datasource.ReadResponse, attes } certBytes, err := cert.MarshalJSON() if err != nil { - resp.Diagnostics.AddError("Marshalling AMD Root Key", err.Error()) + return tfSnpAttestation, err } - tfSnpVersions := sevSnpAttestation{ + tfSnpAttestation = sevSnpAttestation{ BootloaderVersion: snpVersions.Bootloader, TEEVersion: snpVersions.TEE, SNPVersion: snpVersions.SNP, @@ -245,17 +255,20 @@ func convertSNPAttestationTfStateCompatible(resp *datasource.ReadResponse, attes if attestationVariant.Equal(variant.AzureSEVSNP{}) { firmwareCfg := config.DefaultForAzureSEVSNP().FirmwareSignerConfig keyDigestAny, err := firmwareCfg.AcceptedKeyDigests.MarshalYAML() - keyDigest := keyDigestAny.([]string) if err != nil { - resp.Diagnostics.AddError("Marshalling Accepted Key Digests", err.Error()) + return tfSnpAttestation, err + } + keyDigest, ok := keyDigestAny.([]string) + if !ok { + return tfSnpAttestation, errors.New("reading Accepted Key Digests: could not convert to []string") } - tfSnpVersions.AzureSNPFirmwareSignerConfig = azureSnpFirmwareSignerConfig{ + tfSnpAttestation.AzureSNPFirmwareSignerConfig = azureSnpFirmwareSignerConfig{ AcceptedKeyDigests: keyDigest, EnforcementPolicy: firmwareCfg.EnforcementPolicy.String(), MAAURL: firmwareCfg.MAAURL, } } - return tfSnpVersions + return tfSnpAttestation, nil } func convertMeasurementsTfStateCompatible(m measurements.M) map[string]measurement { diff --git a/terraform-provider-constellation/internal/provider/provider.go b/terraform-provider-constellation/internal/provider/provider.go index 203d295190..fdf4967332 100644 --- a/terraform-provider-constellation/internal/provider/provider.go +++ b/terraform-provider-constellation/internal/provider/provider.go @@ -71,7 +71,7 @@ func (p *ConstellationProvider) Configure(ctx context.Context, req provider.Conf config := datastruct.ProviderData{} // Make the clients available during data source and resource "Configure" methods. - // resp.DataSourceData = config // TODO check with moritz + resp.DataSourceData = config resp.ResourceData = config } diff --git a/terraform-provider-constellation/internal/provider/provider_test.go b/terraform-provider-constellation/internal/provider/provider_test.go index b39f682d07..b8b0161e8c 100644 --- a/terraform-provider-constellation/internal/provider/provider_test.go +++ b/terraform-provider-constellation/internal/provider/provider_test.go @@ -34,10 +34,6 @@ var testAccProtoV6ProviderFactories = map[string]func() (tfprotov6.ProviderServe // bazelSetTerraformBinaryPath sets the path to the Terraform binary for // acceptance testing when running under Bazel. func bazelSetTerraformBinaryPath(t *testing.T) { - if !runsUnderBazel() { - return - } - if v := os.Getenv("TF_ACC"); v != "1" { t.Fatal("TF_ACC must be set to \"1\" for acceptance tests") }