Skip to content

Commit

Permalink
Normalize CA strings before comparing them in plans (#106)
Browse files Browse the repository at this point in the history
* Normalize CA strings before comparing them in plans

Terraform, in general, does not approve of situations in which an API returns a value that is not exactly the same as the one that was given to it. In the case of the Temporal Cloud API, the accepted client CA may be returned in a "normalized" form, with whitespace stripped out. This commit implements a semantic equality check for encoded CAs by normalizing base64-encoded CA strings prior to comparing them.

Fixes #102, #90, #87.

* Skip equality check for empty CA strings
  • Loading branch information
swgillespie authored Aug 5, 2024
1 parent 8e7f816 commit de9343a
Show file tree
Hide file tree
Showing 4 changed files with 250 additions and 9 deletions.
20 changes: 11 additions & 9 deletions internal/provider/namespace_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import (
"github.com/hashicorp/terraform-plugin-framework/types/basetypes"

"github.com/temporalio/terraform-provider-temporalcloud/internal/client"
internaltypes "github.com/temporalio/terraform-provider-temporalcloud/internal/types"
cloudservicev1 "github.com/temporalio/terraform-provider-temporalcloud/proto/go/temporal/api/cloud/cloudservice/v1"
namespacev1 "github.com/temporalio/terraform-provider-temporalcloud/proto/go/temporal/api/cloud/namespace/v1"
)
Expand All @@ -56,14 +57,14 @@ type (
}

namespaceResourceModel struct {
ID types.String `tfsdk:"id"`
Name types.String `tfsdk:"name"`
Regions types.List `tfsdk:"regions"`
AcceptedClientCA types.String `tfsdk:"accepted_client_ca"`
RetentionDays types.Int64 `tfsdk:"retention_days"`
CertificateFilters types.List `tfsdk:"certificate_filters"`
CodecServer types.Object `tfsdk:"codec_server"`
Endpoints types.Object `tfsdk:"endpoints"`
ID types.String `tfsdk:"id"`
Name types.String `tfsdk:"name"`
Regions types.List `tfsdk:"regions"`
AcceptedClientCA internaltypes.EncodedCAValue `tfsdk:"accepted_client_ca"`
RetentionDays types.Int64 `tfsdk:"retention_days"`
CertificateFilters types.List `tfsdk:"certificate_filters"`
CodecServer types.Object `tfsdk:"codec_server"`
Endpoints types.Object `tfsdk:"endpoints"`

Timeouts timeouts.Value `tfsdk:"timeouts"`
}
Expand Down Expand Up @@ -166,6 +167,7 @@ func (r *namespaceResource) Schema(ctx context.Context, _ resource.SchemaRequest
},
},
"accepted_client_ca": schema.StringAttribute{
CustomType: internaltypes.EncodedCAType{},
Description: "The Base64-encoded CA cert in PEM format that clients use when authenticating with Temporal Cloud.",
Required: true,
},
Expand Down Expand Up @@ -518,7 +520,7 @@ func updateModelFromSpec(ctx context.Context, diags diag.Diagnostics, state *nam
state.Endpoints = endpointsState
state.Regions = planRegions
state.CertificateFilters = certificateFilter
state.AcceptedClientCA = types.StringValue(ns.GetSpec().GetMtlsAuth().GetAcceptedClientCa())
state.AcceptedClientCA = internaltypes.EncodedCA(ns.GetSpec().GetMtlsAuth().GetAcceptedClientCa())
state.RetentionDays = types.Int64Value(int64(ns.GetSpec().GetRetentionDays()))
}

Expand Down
62 changes: 62 additions & 0 deletions internal/provider/namespace_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,68 @@ PEM
})
}

func TestAccSpacesBetweenCertificateStrings(t *testing.T) {
name := fmt.Sprintf("%s-%s", "tf-basic-namespace", randomString())
config := func(name string, retention int) string {
return fmt.Sprintf(`
provider "temporalcloud" {
}
resource "temporalcloud_namespace" "terraform" {
name = "%s"
regions = ["aws-us-east-1"]
accepted_client_ca = base64encode(<<PEM
-----BEGIN CERTIFICATE-----
MIIByTCCAVCgAwIBAgIRAWHkC+6JUf3s9Tq43mdp2zgwCgYIKoZIzj0EAwMwEzER
MA8GA1UEChMIdGVtcG9yYWwwHhcNMjMwODEwMDAwOTQ1WhcNMjQwODA5MDAxMDQ1
WjATMREwDwYDVQQKEwh0ZW1wb3JhbDB2MBAGByqGSM49AgEGBSuBBAAiA2IABCzQ
7DwwGSQKM6Zrx3Qtw7IubfxiJ3RSXCqmcGhEbFVeocwAdEgMYlwSlUiWtDZVR2dM
XM9UZLWK4aGGnDNS5Mhcz6ibSBS7Owf4tRZZA9SpFCjNw2HraaiUVV+EUgxoe6No
MGYwDgYDVR0PAQH/BAQDAgGGMA8GA1UdEwEB/wQFMAMBAf8wHQYDVR0OBBYEFG4N
8lIXqQKxwVs/ixVzdF6XGZm+MCQGA1UdEQQdMBuCGWNsaWVudC5yb290LnRlbXBv
cmFsLlB1VHMwCgYIKoZIzj0EAwMDZwAwZAIwRLfm9S7rKGd30KdQvUMcOcDJlmDw
6/oM6UOJFxLeGcpYbgxQ/bFize+Yx9Q9kNeMAjA7GiFsaipaKtWHy5MCOCas3ZP6
+ttLaXNXss3Z5Wk5vhDQnyE8JR3rPeQ2cHXLiA0=
-----END CERTIFICATE-----
-----BEGIN CERTIFICATE-----
MIIBxjCCAU2gAwIBAgIRA7oa6vxjwtoTwEv3aVvhehwwCgYIKoZIzj0EAwMwEjEQ
MA4GA1UEChMHdGVzdGluZzAeFw0yNDA4MDIxNjQxNTRaFw0yNTA4MDIxNjQyNTRa
MBIxEDAOBgNVBAoTB3Rlc3RpbmcwdjAQBgcqhkjOPQIBBgUrgQQAIgNiAAQCqUjT
DQUJ7kwkNy+xdsIwN+DchoJbcuePVOEA0yI4t3NcKyCp2RN8dmP3n1buXmUQM80E
llAYMh1GpE7UmhOYviUWzqVj3f7K5Bo0OT/R1qrwqVWW/FomNouYq+z8MLSjZzBl
MA4GA1UdDwEB/wQEAwIBhjAPBgNVHRMBAf8EBTADAQH/MB0GA1UdDgQWBBROU/EH
xCWCZcIzeSrtt4hHWZ3cuTAjBgNVHREEHDAaghhjbGllbnQucm9vdC50ZXN0aW5n
LlFsRFMwCgYIKoZIzj0EAwMDZwAwZAIwVRIHAsna1j7TfyeAdxaCicgM+YGq40U4
Q7c98BVX73Xu6AgYyAUN5xionBYJIBSqAjALqddjqsToi7HW17yI5n7UsIGZlwkq
12okB1IGBRmOi9MRJxnVM0exkXaThHFgKhc=
-----END CERTIFICATE-----
PEM
)
retention_days = %d
}`, name, retention)
}

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
Steps: []resource.TestStep{
{
// New namespace with retention of 7
Config: config(name, 7),
},
{
Config: config(name, 14),
},
// Delete testing automatically occurs in TestCase
},
})
}

func newConnection(t *testing.T) cloudservicev1.CloudServiceClient {
apiKey := os.Getenv("TEMPORAL_CLOUD_API_KEY")
endpoint := os.Getenv("TEMPORAL_CLOUD_ENDPOINT")
Expand Down
161 changes: 161 additions & 0 deletions internal/types/encoded_ca.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
package types

import (
"context"
"crypto/x509"
"encoding/base64"
"encoding/pem"
"errors"
"fmt"
"strings"

"github.com/hashicorp/terraform-plugin-framework/attr"
"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/types/basetypes"
"github.com/hashicorp/terraform-plugin-go/tftypes"
)

var (
_ basetypes.StringTypable = EncodedCAType{}
_ basetypes.StringValuable = EncodedCAValue{}
_ basetypes.StringValuableWithSemanticEquals = EncodedCAValue{}
)

type EncodedCAType struct {
basetypes.StringType
}

func (t EncodedCAType) Equal(o attr.Type) bool {
other, ok := o.(EncodedCAType)
if !ok {
return false
}

return t.StringType.Equal(other.StringType)
}

func (t EncodedCAType) ValueFromString(ctx context.Context, in basetypes.StringValue) (basetypes.StringValuable, diag.Diagnostics) {
return EncodedCA(in.ValueString()), nil
}

func (t EncodedCAType) ValueFromTerraform(ctx context.Context, in tftypes.Value) (attr.Value, error) {
attrValue, err := t.StringType.ValueFromTerraform(ctx, in)
if err != nil {
return nil, err
}

stringValue, ok := attrValue.(basetypes.StringValue)
if !ok {
return nil, errors.New("unexpected value type")
}

stringValuable, diags := t.ValueFromString(ctx, stringValue)
if diags.HasError() {
return nil, fmt.Errorf("unexpected error converting StringValue to StringValuable: %v", diags)
}

return stringValuable, nil
}

type EncodedCAValue struct {
basetypes.StringValue
}

func (v EncodedCAValue) Equal(o attr.Value) bool {
other, ok := o.(EncodedCAValue)
if !ok {
return false
}

return v.StringValue.Equal(other.StringValue)
}

func (v EncodedCAValue) Type(ctx context.Context) attr.Type {
return EncodedCAType{}
}

func EncodedCA(val string) EncodedCAValue {
return EncodedCAValue{
StringValue: basetypes.NewStringValue(val),
}
}

// normalizeCAString accepts a base64-encoded PEM string and normalizes it by removing extraneous whitespace. The
// Temporal API will do this and cause a mismatch in the state if we don't also do it here.
func normalizeCAString(certPEMBase64 string) (string, error) {
certs, err := parseEncodedCertificates(certPEMBase64)
if err != nil {
return "", err
}

var result []byte
for _, c := range certs {
pemBytes := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: c.Raw})
result = append(result, pemBytes...)
}

return base64.StdEncoding.EncodeToString(result), nil
}

func parseEncodedCertificates(certPEMBase64 string) ([]*x509.Certificate, error) {
certPEMBytes, err := base64.StdEncoding.DecodeString(certPEMBase64)
if err != nil {
return nil, fmt.Errorf("failed to decode base64-encoded PEM: %w", err)
}

if len(certPEMBytes) == 0 {
return nil, fmt.Errorf("no certificates received")
}

var blocks []byte
cursor := certPEMBytes
for {
var block *pem.Block
block, cursor = pem.Decode(cursor)
if block == nil {
break
}

blocks = append(blocks, block.Bytes...)
}

// If p is greater than 0, then this means that there was a portion of the certificate that
// is/was malformed.
if len(cursor) > 0 && strings.TrimSpace(string(cursor)) != "" {
return []*x509.Certificate{}, errors.New("malformed certificates")
}
return x509.ParseCertificates(blocks)
}

func (v EncodedCAValue) StringSemanticEquals(ctx context.Context, newValuable basetypes.StringValuable) (bool, diag.Diagnostics) {
var diags diag.Diagnostics

newValue, ok := newValuable.(EncodedCAValue)
if !ok {
diags.AddError(
"Semantic Equality Check Error",
"An unexpected value type was received while performing semantic equality checks. "+
"Please report this to the provider developers.\n\n"+
"Expected Value Type: "+fmt.Sprintf("%T", v)+"\n"+
"Got Value Type: "+fmt.Sprintf("%T", newValuable),
)

return false, diags
}

// Normalize the certificate strings before comparing them.
normalizedV, err := normalizeCAString(v.ValueString())
if err != nil {
diags.AddError("Certificate Normalization Error", "Failed to normalize the existing certificate: "+err.Error())
return false, diags
}

normalizedNewValue, err := normalizeCAString(newValue.ValueString())
if err != nil {
// The new value may not be a valid CA string. This will get rejected elsewhere in the plan. Since this is just
// an equality check, we should return false and continue.
return false, diags
}

return normalizedV == normalizedNewValue, diags
}
16 changes: 16 additions & 0 deletions internal/types/encoded_ca_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package types

import "testing"

func TestCACertNormalization(t *testing.T) {
input := "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUJ5VENDQVZDZ0F3SUJBZ0lSQVdIa0MrNkpVZjNzOVRxNDNtZHAyemd3Q2dZSUtvWkl6ajBFQXdNd0V6RVIKTUE4R0ExVUVDaE1JZEdWdGNHOXlZV3d3SGhjTk1qTXdPREV3TURBd09UUTFXaGNOTWpRd09EQTVNREF4TURRMQpXakFUTVJFd0R3WURWUVFLRXdoMFpXMXdiM0poYkRCMk1CQUdCeXFHU000OUFnRUdCU3VCQkFBaUEySUFCQ3pRCjdEd3dHU1FLTTZacngzUXR3N0l1YmZ4aUozUlNYQ3FtY0doRWJGVmVvY3dBZEVnTVlsd1NsVWlXdERaVlIyZE0KWE05VVpMV0s0YUdHbkROUzVNaGN6NmliU0JTN093ZjR0UlpaQTlTcEZDak53MkhyYWFpVVZWK0VVZ3hvZTZObwpNR1l3RGdZRFZSMFBBUUgvQkFRREFnR0dNQThHQTFVZEV3RUIvd1FGTUFNQkFmOHdIUVlEVlIwT0JCWUVGRzROCjhsSVhxUUt4d1ZzL2l4VnpkRjZYR1ptK01DUUdBMVVkRVFRZE1CdUNHV05zYVdWdWRDNXliMjkwTG5SbGJYQnYKY21Gc0xsQjFWSE13Q2dZSUtvWkl6ajBFQXdNRFp3QXdaQUl3UkxmbTlTN3JLR2QzMEtkUXZVTWNPY0RKbG1Edwo2L29NNlVPSkZ4TGVHY3BZYmd4US9iRml6ZStZeDlROWtOZU1BakE3R2lGc2FpcGFLdFdIeTVNQ09DYXMzWlA2Cit0dExhWE5Yc3MzWjVXazV2aERRbnlFOEpSM3JQZVEyY0hYTGlBMD0KLS0tLS1FTkQgQ0VSVElGSUNBVEUtLS0tLQoKCi0tLS0tQkVHSU4gQ0VSVElGSUNBVEUtLS0tLQpNSUlCeGpDQ0FVMmdBd0lCQWdJUkE3b2E2dnhqd3RvVHdFdjNhVnZoZWh3d0NnWUlLb1pJemowRUF3TXdFakVRCk1BNEdBMVVFQ2hNSGRHVnpkR2x1WnpBZUZ3MHlOREE0TURJeE5qUXhOVFJhRncweU5UQTRNREl4TmpReU5UUmEKTUJJeEVEQU9CZ05WQkFvVEIzUmxjM1JwYm1jd2RqQVFCZ2NxaGtqT1BRSUJCZ1VyZ1FRQUlnTmlBQVFDcVVqVApEUVVKN2t3a055K3hkc0l3TitEY2hvSmJjdWVQVk9FQTB5STR0M05jS3lDcDJSTjhkbVAzbjFidVhtVVFNODBFCmxsQVlNaDFHcEU3VW1oT1l2aVVXenFWajNmN0s1Qm8wT1QvUjFxcndxVldXL0ZvbU5vdVlxK3o4TUxTalp6QmwKTUE0R0ExVWREd0VCL3dRRUF3SUJoakFQQmdOVkhSTUJBZjhFQlRBREFRSC9NQjBHQTFVZERnUVdCQlJPVS9FSAp4Q1dDWmNJemVTcnR0NGhIV1ozY3VUQWpCZ05WSFJFRUhEQWFnaGhqYkdsbGJuUXVjbTl2ZEM1MFpYTjBhVzVuCkxsRnNSRk13Q2dZSUtvWkl6ajBFQXdNRFp3QXdaQUl3VlJJSEFzbmExajdUZnllQWR4YUNpY2dNK1lHcTQwVTQKUTdjOThCVlg3M1h1NkFnWXlBVU41eGlvbkJZSklCU3FBakFMcWRkanFzVG9pN0hXMTd5STVuN1VzSUdabHdrcQoxMm9rQjFJR0JSbU9pOU1SSnhuVk0wZXhrWGFUaEhGZ0toYz0KLS0tLS1FTkQgQ0VSVElGSUNBVEUtLS0tLQ=="
expected := "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUJ5VENDQVZDZ0F3SUJBZ0lSQVdIa0MrNkpVZjNzOVRxNDNtZHAyemd3Q2dZSUtvWkl6ajBFQXdNd0V6RVIKTUE4R0ExVUVDaE1JZEdWdGNHOXlZV3d3SGhjTk1qTXdPREV3TURBd09UUTFXaGNOTWpRd09EQTVNREF4TURRMQpXakFUTVJFd0R3WURWUVFLRXdoMFpXMXdiM0poYkRCMk1CQUdCeXFHU000OUFnRUdCU3VCQkFBaUEySUFCQ3pRCjdEd3dHU1FLTTZacngzUXR3N0l1YmZ4aUozUlNYQ3FtY0doRWJGVmVvY3dBZEVnTVlsd1NsVWlXdERaVlIyZE0KWE05VVpMV0s0YUdHbkROUzVNaGN6NmliU0JTN093ZjR0UlpaQTlTcEZDak53MkhyYWFpVVZWK0VVZ3hvZTZObwpNR1l3RGdZRFZSMFBBUUgvQkFRREFnR0dNQThHQTFVZEV3RUIvd1FGTUFNQkFmOHdIUVlEVlIwT0JCWUVGRzROCjhsSVhxUUt4d1ZzL2l4VnpkRjZYR1ptK01DUUdBMVVkRVFRZE1CdUNHV05zYVdWdWRDNXliMjkwTG5SbGJYQnYKY21Gc0xsQjFWSE13Q2dZSUtvWkl6ajBFQXdNRFp3QXdaQUl3UkxmbTlTN3JLR2QzMEtkUXZVTWNPY0RKbG1Edwo2L29NNlVPSkZ4TGVHY3BZYmd4US9iRml6ZStZeDlROWtOZU1BakE3R2lGc2FpcGFLdFdIeTVNQ09DYXMzWlA2Cit0dExhWE5Yc3MzWjVXazV2aERRbnlFOEpSM3JQZVEyY0hYTGlBMD0KLS0tLS1FTkQgQ0VSVElGSUNBVEUtLS0tLQotLS0tLUJFR0lOIENFUlRJRklDQVRFLS0tLS0KTUlJQnhqQ0NBVTJnQXdJQkFnSVJBN29hNnZ4and0b1R3RXYzYVZ2aGVod3dDZ1lJS29aSXpqMEVBd013RWpFUQpNQTRHQTFVRUNoTUhkR1Z6ZEdsdVp6QWVGdzB5TkRBNE1ESXhOalF4TlRSYUZ3MHlOVEE0TURJeE5qUXlOVFJhCk1CSXhFREFPQmdOVkJBb1RCM1JsYzNScGJtY3dkakFRQmdjcWhrak9QUUlCQmdVcmdRUUFJZ05pQUFRQ3FValQKRFFVSjdrd2tOeSt4ZHNJd04rRGNob0piY3VlUFZPRUEweUk0dDNOY0t5Q3AyUk44ZG1QM24xYnVYbVVRTTgwRQpsbEFZTWgxR3BFN1VtaE9ZdmlVV3pxVmozZjdLNUJvME9UL1IxcXJ3cVZXVy9Gb21Ob3VZcSt6OE1MU2paekJsCk1BNEdBMVVkRHdFQi93UUVBd0lCaGpBUEJnTlZIUk1CQWY4RUJUQURBUUgvTUIwR0ExVWREZ1FXQkJST1UvRUgKeENXQ1pjSXplU3J0dDRoSFdaM2N1VEFqQmdOVkhSRUVIREFhZ2hoamJHbGxiblF1Y205dmRDNTBaWE4wYVc1bgpMbEZzUkZNd0NnWUlLb1pJemowRUF3TURad0F3WkFJd1ZSSUhBc25hMWo3VGZ5ZUFkeGFDaWNnTStZR3E0MFU0ClE3Yzk4QlZYNzNYdTZBZ1l5QVVONXhpb25CWUpJQlNxQWpBTHFkZGpxc1RvaTdIVzE3eUk1bjdVc0lHWmx3a3EKMTJva0IxSUdCUm1PaTlNUkp4blZNMGV4a1hhVGhIRmdLaGM9Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K"
normalized, err := normalizeCAString(input)
if err != nil {
t.Fatalf("failed to normalize cert: %v", err)
}

if normalized != expected {
t.Fatalf("unexpected normalized cert: %s", normalized)
}
}

0 comments on commit de9343a

Please sign in to comment.