Support changing property types of port_blueprint (#255)

* Support changing property types of port_blueprint

Added a flag that represents our current protection against changing the property type.

When the flag is set to false, we delete the property before recreating it with the new type.

* update docs

* split cover report to a different job

* Add support for changing property type of system blueprints

* change needs of public_cover_report from public_report to acctest

* add checkout step to publish_cover_report

* clean state after test

* Remove unused PatchBlueprint method

Author: Eyal-Shalev

.github/workflows/ci.yml

@@ -152,6 +152,14 @@ jobs:
           flaky_summary: 'true'
           include_time_in_summary: 'true'
           simplified_summary: 'true'
+
+  publish_cover_report:
+    name: Publish Coverage Report
+    if: ${{ always() }}
+    needs: acctest
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
       - name: Download all coverage reports
         uses: actions/download-artifact@v4
         with:

docs/index.md

@@ -18,6 +18,7 @@ Interact with Port-labs
 ### Optional
 
 - `base_url` (String)
+- `blueprint_property_type_change_protection` (Boolean) Protects you from accidentally changing the property type of blueprints which will delete the property before recreating it with the new type. Defaults to `true`
 - `client_id` (String) Client ID for Port-labs
 - `json_escape_html` (Boolean) When set to `false` disables the default HTML escaping of json.Marshal when reading data from Port. Defaults to `true`
 - `secret` (String, Sensitive) Client Secret for Port-labs

internal/acctest/acctest.go

@@ -28,6 +28,14 @@ var ProviderConfig = fmt.Sprintf(`provider "port" {
 	}
 `, os.Getenv("PORT_CLIENT_ID"), os.Getenv("PORT_CLIENT_SECRET"), os.Getenv("PORT_BASE_URL"))
 
+var ProviderConfigNoPropertyTypeProtection = fmt.Sprintf(`provider "port" {
+	client_id = "%s"
+	secret = "%s"
+	base_url = "%s"
+	blueprint_property_type_change_protection = false
+	}
+`, os.Getenv("PORT_CLIENT_ID"), os.Getenv("PORT_CLIENT_SECRET"), os.Getenv("PORT_BASE_URL"))
+
 var ProviderConfigNoEscapeHTML = fmt.Sprintf(`provider "port" {
 	client_id = "%s"
 	secret = "%s"

internal/cli/blueprint.go

@@ -8,7 +8,7 @@ import (
 
 func (c *PortClient) ReadBlueprint(ctx context.Context, id string) (*Blueprint, int, error) {
 	pb := &PortBody{}
-	url := "v1/blueprints/{identifier}"
+	const url = "v1/blueprints/{identifier}"
 	resp, err := c.Client.R().
 		SetContext(ctx).
 		SetHeader("Accept", "application/json").
@@ -27,7 +27,7 @@ func (c *PortClient) ReadBlueprint(ctx context.Context, id string) (*Blueprint,
 
 func (c *PortClient) ReadSystemBlueprintStructure(ctx context.Context, id string) (*Blueprint, int, error) {
 	pb := &PortBody{}
-	url := "v1/blueprints/system/{identifier}/structure"
+	const url = "v1/blueprints/system/{identifier}/structure"
 	resp, err := c.Client.R().
 		SetContext(ctx).
 		SetHeader("Accept", "application/json").
@@ -44,7 +44,7 @@ func (c *PortClient) ReadSystemBlueprintStructure(ctx context.Context, id string
 }
 
 func (c *PortClient) CreateBlueprint(ctx context.Context, b *Blueprint, createCatalogPage *bool) (*Blueprint, error) {
-	url := "v1/blueprints"
+	const url = "v1/blueprints"
 	request := c.Client.R().
 		SetBody(b).
 		SetContext(ctx)
@@ -67,7 +67,7 @@ func (c *PortClient) CreateBlueprint(ctx context.Context, b *Blueprint, createCa
 }
 
 func (c *PortClient) UpdateBlueprint(ctx context.Context, b *Blueprint, id string) (*Blueprint, error) {
-	url := "v1/blueprints/{identifier}"
+	const url = "v1/blueprints/{identifier}"
 	resp, err := c.Client.R().
 		SetBody(b).
 		SetContext(ctx).
@@ -88,7 +88,7 @@ func (c *PortClient) UpdateBlueprint(ctx context.Context, b *Blueprint, id strin
 }
 
 func (c *PortClient) DeleteBlueprint(ctx context.Context, id string) error {
-	url := "v1/blueprints/{identifier}"
+	const url = "v1/blueprints/{identifier}"
 	resp, err := c.Client.R().
 		SetContext(ctx).
 		SetHeader("Accept", "application/json").
@@ -109,7 +109,7 @@ func (c *PortClient) DeleteBlueprint(ctx context.Context, id string) error {
 }
 
 func (c *PortClient) DeleteBlueprintWithAllEntities(ctx context.Context, id string) (*string, error) {
-	url := "v1/blueprints/{identifier}/all-entities?delete_blueprint=true"
+	const url = "v1/blueprints/{identifier}/all-entities?delete_blueprint=true"
 	resp, err := c.Client.R().
 		SetContext(ctx).
 		SetHeader("Accept", "application/json").
@@ -128,5 +128,4 @@ func (c *PortClient) DeleteBlueprintWithAllEntities(ctx context.Context, id stri
 	}
 
 	return &pb.MigrationId, nil
-
 }

internal/cli/client.go

@@ -13,11 +13,12 @@ import (
 type Option func(*PortClient)
 
 type PortClient struct {
-	Client         *resty.Client
-	ClientID       string
-	Token          string
-	featureFlags   []string
-	JSONEscapeHTML bool
+	Client                                *resty.Client
+	ClientID                              string
+	Token                                 string
+	featureFlags                          []string
+	JSONEscapeHTML                        bool
+	BlueprintPropertyTypeChangeProtection bool
 }
 
 func New(baseURL string, opts ...Option) (*PortClient, error) {

internal/cli/models.go

@@ -548,11 +548,12 @@ type PortTeamBody struct {
 }
 
 type PortProviderModel struct {
-	ClientId       types.String `tfsdk:"client_id"`
-	Secret         types.String `tfsdk:"secret"`
-	Token          types.String `tfsdk:"token"`
-	BaseUrl        types.String `tfsdk:"base_url"`
-	JSONEscapeHTML types.Bool   `tfsdk:"json_escape_html"`
+	ClientId                              types.String `tfsdk:"client_id"`
+	Secret                                types.String `tfsdk:"secret"`
+	Token                                 types.String `tfsdk:"token"`
+	BaseUrl                               types.String `tfsdk:"base_url"`
+	JSONEscapeHTML                        types.Bool   `tfsdk:"json_escape_html"`
+	BlueprintPropertyTypeChangeProtection types.Bool   `tfsdk:"blueprint_property_type_change_protection"`
 }
 
 type PortBodyDelete struct {

port/blueprint/resource.go

@@ -188,27 +188,33 @@ func (r *BlueprintResource) Update(ctx context.Context, req resource.UpdateReque
 
 	resp.Diagnostics.Append(req.Plan.Get(ctx, &state)...)
 	resp.Diagnostics.Append(req.State.Get(ctx, &previousState)...)
-
 	if resp.Diagnostics.HasError() {
 		return
 	}
-	b, err := blueprintResourceToPortRequest(ctx, state)
 
+	prevB, err := blueprintResourceToPortRequest(ctx, previousState)
+	if err != nil {
+		resp.Diagnostics.AddError("failed to transform previous state into a blueprint", err.Error())
+		return
+	}
+
+	b, err := blueprintResourceToPortRequest(ctx, state)
 	if err != nil {
 		resp.Diagnostics.AddError("failed to transform blueprint", err.Error())
 		return
 	}
 
 	var bp *cli.Blueprint
-	createCatalogPage := state.CreateCatalogPage.ValueBoolPointer()
 	if previousState.Identifier.IsNull() {
-		bp, err = r.portClient.CreateBlueprint(ctx, b, createCatalogPage)
+		bp, err = r.portClient.CreateBlueprint(ctx, b, state.CreateCatalogPage.ValueBoolPointer())
 		if err != nil {
 			resp.Diagnostics.AddError("failed to create blueprint", err.Error())
 			return
 		}
 	} else {
-		existingBp, statusCode, err := r.portClient.ReadBlueprint(ctx, previousState.Identifier.ValueString())
+		var existingBp *cli.Blueprint
+		var statusCode int
+		existingBp, statusCode, err = r.portClient.ReadBlueprint(ctx, previousState.Identifier.ValueString())
 		if err != nil {
 			if statusCode == 404 {
 				resp.Diagnostics.AddError("Blueprint doesn't exists, it is required to update the blueprint", err.Error())
@@ -220,6 +226,38 @@ func (r *BlueprintResource) Update(ctx context.Context, req resource.UpdateReque
 		// aggregation properties are managed in a different resource, so we need to keep them in the update
 		// to avoid losing them
 		b.AggregationProperties = existingBp.AggregationProperties
+		prevB.AggregationProperties = existingBp.AggregationProperties
+
+		propsWithChangedTypes := make(map[string]string, 0)
+		for propKey, prop := range b.Schema.Properties {
+			if prevProp, prevHasProp := prevB.Schema.Properties[propKey]; prevHasProp && prop.Type != prevProp.Type {
+				propsWithChangedTypes[propKey] = prevProp.Type
+				delete(prevB.Schema.Properties, propKey)
+			}
+		}
+		if len(propsWithChangedTypes) > 0 && r.portClient.BlueprintPropertyTypeChangeProtection {
+			for propKey, prevPropType := range propsWithChangedTypes {
+				currentPropType := b.Schema.Properties[propKey].Type
+				resp.Diagnostics.AddAttributeError(
+					path.Root("properties").AtName(fmt.Sprintf("%s_props", currentPropType)).
+						AtName(propKey).AtName("type"),
+					"Property type changed while protection is enabled",
+					fmt.Sprintf("The type of property %q changed from %q to %q. Applying this change will cause "+
+						"you to lose the data for that property. If you wish to continue disable the protection in the "+
+						"provider configuration by setting %q to false", propKey, prevPropType, currentPropType,
+						"blueprint_property_type_change_protection"),
+				)
+			}
+			return
+		}
+		if len(propsWithChangedTypes) > 0 {
+			_, err = r.portClient.UpdateBlueprint(ctx, prevB, previousState.ID.ValueString())
+			if err != nil {
+				resp.Diagnostics.AddError("failed to pre-delete properties that changed their type", err.Error())
+				return
+			}
+		}
+
 		bp, err = r.portClient.UpdateBlueprint(ctx, b, previousState.ID.ValueString())
 		if err != nil {
 			resp.Diagnostics.AddError("failed to update blueprint", err.Error())

port/blueprint/resource_test.go

@@ -3,11 +3,15 @@ package blueprint_test
 import (
 	"context"
 	"fmt"
+	"github.com/stretchr/testify/require"
+	"math/rand"
 	"net/http"
 	"os"
 	"regexp"
 	"strconv"
+	"strings"
 	"testing"
+	"text/template"
 	"time"
 
 	"github.com/port-labs/terraform-provider-port-labs/v2/internal/cli"
@@ -324,6 +328,95 @@ func TestAccPortBlueprintObjectProperty(t *testing.T) {
 	})
 }
 
+func TestAccPortBlueprintChangePropertyType(t *testing.T) {
+	type data struct{ Identifier, PropType string }
+	identifier := utils.GenID()
+	tmpl, err := template.New("resource").Parse(`
+	resource "port_blueprint" "{{.Identifier}}" {
+		title = "test: {{.Identifier}}"
+		icon = "Terraform"
+		identifier = "{{.Identifier}}"
+		properties = {
+			{{.PropType}}_props = {
+				myProperty = {
+					title = "My Property"
+					description = "This is a {{.PropType}} property"
+				}
+			}
+		}
+	}`)
+	require.NoErrorf(t, err, "failed to parse test template")
+
+	var propTypes = [...]string{"string", "number", "boolean", "array", "object"}
+
+	// Shuffle the prop types to make sure we don't have an issue transitioning from one type to the next.
+	rand.Shuffle(len(propTypes), func(i, j int) { propTypes[i], propTypes[j] = propTypes[j], propTypes[i] })
+
+	steps := make([]resource.TestStep, len(propTypes))
+	for idx, propType := range propTypes {
+		var txt strings.Builder
+		err = tmpl.Execute(&txt, data{Identifier: identifier, PropType: propType})
+		require.NoErrorf(t, err, "failed to execute template for propType: %s", propType)
+		steps[idx] = resource.TestStep{
+			Config: acctest.ProviderConfigNoPropertyTypeProtection + txt.String(),
+			Check: resource.ComposeTestCheckFunc(resource.TestCheckResourceAttr(
+				fmt.Sprintf("port_blueprint.%s", identifier),
+				fmt.Sprintf("properties.%s_props.myProperty.title", propType),
+				"My Property",
+			)),
+		}
+	}
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:                 func() { acctest.TestAccPreCheck(t) },
+		ProtoV6ProviderFactories: acctest.TestAccProtoV6ProviderFactories,
+		Steps:                    steps,
+	})
+}
+
+func TestAccPortBlueprintChangePropertyTypeProtection(t *testing.T) {
+	type data struct{ Identifier, PropType string }
+	identifier := utils.GenID()
+	tmpl, err := template.New("resource").Parse(`
+	resource "port_blueprint" "{{.Identifier}}" {
+		title = "test: {{.Identifier}}"
+		icon = "Terraform"
+		identifier = "{{.Identifier}}"
+		properties = {
+			{{.PropType}}_props = {
+				myProperty = {
+					title = "My Property"
+					description = "This is a {{.PropType}} property"
+				}
+			}
+		}
+	}`)
+	require.NoErrorf(t, err, "failed to parse test template")
+
+	var step1Text, step2Text strings.Builder
+	require.NoError(t, tmpl.Execute(&step1Text, data{Identifier: identifier, PropType: "string"}))
+	require.NoError(t, tmpl.Execute(&step2Text, data{Identifier: identifier, PropType: "number"}))
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:                 func() { acctest.TestAccPreCheck(t) },
+		ProtoV6ProviderFactories: acctest.TestAccProtoV6ProviderFactories,
+		Steps: []resource.TestStep{
+			{
+				Config: acctest.ProviderConfig + step1Text.String(),
+				Check: resource.ComposeTestCheckFunc(resource.TestCheckResourceAttr(
+					fmt.Sprintf("port_blueprint.%s", identifier),
+					"properties.string_props.myProperty.title",
+					"My Property",
+				)),
+			},
+			{
+				Config:      acctest.ProviderConfig + step2Text.String(),
+				ExpectError: regexp.MustCompile(`The type of property "myProperty" changed from "string" to "number"`),
+			},
+		},
+	})
+}
+
 func TestAccPortBlueprintWithChangelogDestination(t *testing.T) {
 	identifier := utils.GenID()
 	identifier2 := utils.GenID()