Code review

Author: dominikhuembeli

.github/workflows/test.yml

@@ -40,7 +40,7 @@ jobs:
       - run: make tfdocs-generate
       - name: Comparing generated Terraform documentation against main
         run: |
-          git diff --compact-summary --exit-code || \
+          git diff --exit-code || \
             (echo; echo "Unexpected difference in directories after code generation. Run 'go generate ./...' command and commit."; exit 1)
 
   test:

.gitignore

@@ -1,35 +1,35 @@
-*.dll
-*.exe
-.DS_Store
-example.tf
-terraform.tfplan
-terraform.tfstate
-bin/
-dist/
-modules-dev/
-/pkg/
-website/.vagrant
-website/.bundle
-website/build
-website/node_modules
-.vagrant/
-*.backup
-./*.tfstate
-.terraform/
-*.log
-*.bak
-*~
-.*.swp
-.idea
-*.iml
-*.test
-*.iml
-
-website/vendor
-
-# Test exclusions
-!command/test-fixtures/**/*.tfstate
-!command/test-fixtures/**/.terraform/
-
-# Keep windows files with windows line endings
-*.winfile eol=crlf
+*.dll
+*.exe
+.DS_Store
+example.tf
+terraform.tfplan
+terraform.tfstate
+bin/
+dist/
+modules-dev/
+/pkg/
+website/.vagrant
+website/.bundle
+website/build
+website/node_modules
+.vagrant/
+*.backup
+./*.tfstate
+.terraform/
+*.log
+*.bak
+*~
+.*.swp
+.idea
+*.iml
+*.test
+*.iml
+
+website/vendor
+
+# Test exclusions
+!command/test-fixtures/**/*.tfstate
+!command/test-fixtures/**/.terraform/
+
+# Keep windows files with windows line endings
+*.winfile eol=crlf

.golangci.yml

@@ -1,26 +1,26 @@
-# Visit https://golangci-lint.run/ for usage documentation
-# and information on other useful linters
-issues:
-  max-per-linter: 0
-  max-same-issues: 0
-
-linters:
-  disable-all: true
-  enable:
-    - durationcheck
-    - errcheck
-    - exportloopref
-    - forcetypeassert
-    - godot
-    - gosimple
-    - ineffassign
-    - makezero
-    - misspell
-    - nilerr
-    - predeclared
-    - staticcheck
-    - tenv
-    - unconvert
-    - unparam
-    - unused
-    - vet
+# Visit https://golangci-lint.run/ for usage documentation
+# and information on other useful linters
+issues:
+  max-per-linter: 0
+  max-same-issues: 0
+
+linters:
+  disable-all: true
+  enable:
+    - durationcheck
+    - errcheck
+    - exportloopref
+    - forcetypeassert
+    - godot
+    - gosimple
+    - ineffassign
+    - makezero
+    - misspell
+    - nilerr
+    - predeclared
+    - staticcheck
+    - tenv
+    - unconvert
+    - unparam
+    - unused
+    - vet

docs/guides/howto-uselocalprovider.md

@@ -8,21 +8,18 @@ page_title: "Use local provider in terraform root moduls"
 
 Use the local version of the terraform provider plugin to test the changes in an real life terraform root module using this provider.
 
-## Step-By-Step
+1. Navigate to the root directory of the terraform provider and build the provider using the command `go build`. Make sure to run this command after every update to the provider.
 
-1. Depending on the OS create the following file (see [CLI Configuration](https://developer.hashicorp.com/terraform/cli/config/config-file#locations))
+2. Depending on your operating system, create the appropriate configuration file for the CLI. On Linux/Apple, create `~/.terraformrc`. On Windows, create `%APPDATA\terraform.rc`. Inside the configuration file, add the following code:
 
-   - On Linux/Apple `~/.terraformrc`
-   - On Windows `%APPDATA\terraform.rc`
+```hcl
+provider_installation {
+  dev_overrides {
+   "DigitecGalaxus/dg-servicebus" = "/mnt/c/Developement/terraform-provider-dg-servicebus"
+  }
+}
+```
 
-   ```hcl
-   provider_installation {
-     dev_overrides {
-       "DigitecGalaxus/dg-servicebus" = "/mnt/c/Developement/terraform-provider-dg-servicebus"
-     }
-   }
-   ```
+3. Navigate to the root directory of the desired root module and run `terraform init -reconfigure`. You should see a warning message in the output indicating that the provider has been overwritten.
 
-2. Go the root directory of the root module you want to deploy and run `terraform init -reconfigure`. Your should see the follwing message in the output:
-
-   ![Provider overwrite warning](provider_overwrite.jpg)
+![Provider overwrite warning](provider_overwrite.jpg)

docs/resources/endpoint.md

@@ -22,8 +22,8 @@ resource "dgservicebus_endpoint" "example" {
     "Dg.SalesOrder.V1.AAA",
   ]
   queue_options = {
-    enable_partitioning   = false,
-    max_size_in_megabytes = 5120,
+    enable_partitioning           = false,
+    max_size_in_megabytes         = 5120,
     max_message_size_in_kilobytes = 256,
   }
 }

examples/provider/provider.tf

@@ -1,15 +1,15 @@
-terraform {
-  required_providers {
-    dgservicebus = {
-      source  = "DigitecGalaxus/dg-servicebus"
-      version = ">= 1.0.0, < 2.0.0"
-    }
-  }
-}
-
-provider "dgservicebus" {
-  azure_servicebus_hostname = "nservicebus.servicebus.windows.net"
-  tenant_id                 = var.tenant_id
-  client_id                 = var.client_id
-  client_secret             = var.client_secret
-}
+terraform {
+  required_providers {
+    dgservicebus = {
+      source  = "DigitecGalaxus/dg-servicebus"
+      version = ">= 1.0.0, < 2.0.0"
+    }
+  }
+}
+
+provider "dgservicebus" {
+  azure_servicebus_hostname = "nservicebus.servicebus.windows.net"
+  tenant_id                 = var.tenant_id
+  client_id                 = var.client_id
+  client_secret             = var.client_secret
+}

examples/resources/dgservicebus_endpoint/resource.tf

@@ -7,8 +7,8 @@ resource "dgservicebus_endpoint" "example" {
     "Dg.SalesOrder.V1.AAA",
   ]
   queue_options = {
-    enable_partitioning   = false,
-    max_size_in_megabytes = 5120,
+    enable_partitioning           = false,
+    max_size_in_megabytes         = 5120,
     max_message_size_in_kilobytes = 256,
   }
 }

internal/provider/asb/client_wrapper.go

@@ -1,9 +1,9 @@
-package asb
-
-import (
-	az "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/admin"
-)
-
-type AsbClientWrapper struct {
-	Client *az.Client
-}
+package asb
+
+import (
+	az "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/admin"
+)
+
+type AsbClientWrapper struct {
+	Client *az.Client
+}

internal/provider/asb/endpoint_subscription.go

@@ -62,14 +62,14 @@ func (w *AsbClientWrapper) GetAsbSubscriptionsRules(
 
 func convertToAsbSubscriptionRule(rule az.RuleProperties) (*AsbSubscriptionRule, error) {
 	if ruleFilter, ok := rule.Filter.(*az.CorrelationFilter); ok {
-		ruleFilterValus, ok := ruleFilter.ApplicationProperties[CORRELATIONFILTER_HEADER].(string)
+		ruleFilterValue, ok := ruleFilter.ApplicationProperties[CORRELATIONFILTER_HEADER].(string)
 		if !ok {
 			return nil, fmt.Errorf("rule filter could not be converted to string")
 		}
 
 		return &AsbSubscriptionRule{
 			Name:       rule.Name,
-			Filter:     ruleFilterValus,
+			Filter:     ruleFilterValue,
 			FilterType: "correlation",
 		}, nil
 	}

internal/provider/endpoint/endpoint_resource_import.go

@@ -2,42 +2,13 @@ package endpoint
 
 import (
 	"context"
-	"fmt"
-	"strings"
-	"terraform-provider-dg-servicebus/internal/provider/asb"
 
-	"github.com/hashicorp/terraform-plugin-framework/path"
 	"github.com/hashicorp/terraform-plugin-framework/resource"
 )
 
 func (r *endpointResource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) {
-	idParts := strings.Split(req.ID, ",")
-	hasNonEmptySubscriptionArgument := len(idParts) == 3 && idParts[2] != ""
-	hasNoSubscriptionArgument := len(idParts) == 2
-	hasEmptyIdValues := len(idParts) >= 2 && idParts[0] == "" && idParts[1] == ""
-
-	if !(hasNonEmptySubscriptionArgument || hasNoSubscriptionArgument) || hasEmptyIdValues {
-		resp.Diagnostics.AddWarning("Foo", fmt.Sprintf("%t %t %t", hasNonEmptySubscriptionArgument, hasNoSubscriptionArgument, hasEmptyIdValues))
-		resp.Diagnostics.AddError(
-			"Unexpected import Identifier",
-			fmt.Sprintf(
-				"Expected two or three identifier in format: topic_name,endpoint_name,<SubscriptionOne;SubscriptionTwo>. Got %q",
-				req.ID,
-			),
-		)
-
-		return
-	}
-
-	model := asb.AsbEndpointModel{TopicName: idParts[0], EndpointName: idParts[1]}
-
-	subscriptionNames := []string{}
-	if hasNonEmptySubscriptionArgument {
-		subscriptionNames = strings.Split(idParts[2], ";")
-	}
-
-	resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("topic_name"), model.TopicName)...)
-	resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("endpoint_name"), model.EndpointName)...)
-	resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("queue_options"), &endpointResourceQueueOptionsModel{})...)
-	resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("subscriptions"), subscriptionNames)...)
+	resp.Diagnostics.AddError(
+		"Import not supported yet",
+		"Import is not supported for this resource",
+	)
 }

internal/provider/endpoint/endpoint_resource_update.go

@@ -78,10 +78,6 @@ func (r *endpointResource) UpdateSubscriptions(
 	resp *resource.UpdateResponse,
 ) error {
 	planModel := plan.ToAsbModel()
-
-	// tflog.Info(ctx, fmt.Sprintf("Previous state: %s", strings.Join(previousState.Subscriptions, ", ")))
-	// tflog.Info(ctx, fmt.Sprintf("Plan: %s", strings.Join(plan.Subscriptions, ", ")))
-
 	// This is deliberately done in this order, such that if subscriptions are replaced,
 	// the new subscriptions are created before the old ones are deleted, thus avoiding
 	// the Endpoint missing events for a short period of time.
@@ -104,6 +100,8 @@ func (r *endpointResource) UpdateSubscriptions(
 			if err == nil {
 				continue
 			}
+
+			return err
 		}
 
 		if rule.FilterType != planSubscription.FilterType.ValueString() {