remove changes to client.go and other review comments.

Co-authored-by: Dan Leehr <[email protected]>

Author: arrestle

Makefile

@@ -9,25 +9,25 @@ build: ## Compile the Go package. This is the default.
 	@echo "==> Building package..."
 	go build
 
-test: ## Execute all unit tests with verbose output."
+clean: ## Remove the .tools folder and binaries.
+	@rm -rf .tools
+	@rm terraform-provider-aap
+
+test: ## Execute all unit tests with verbose output.
 	@echo "==> Running unit tests..."
 	go test -v ./...
 
-testacc: ## Run acceptance tests against local aap-dev instance (https://localhost:8043)."
-	@echo "==> Running acceptance tests..."
-	TF_ACC=1 AAP_HOST="http://localhost:8043" go test -count=1 -v ./...
-
-testacc-aapdev: ## Run acceptance tests against local aap-dev instance (EXPORT AAP_HOST="http://localhost:9080")
+testacc: ## Run Acceptance tests against aap instance (See README.md for env variables)
 	@echo "==> Running acceptance tests..."
 	TF_ACC=1 go test -count=1 -v ./...
 
-generatedocs: ## Format example Terraform configurations and generate plugin documentation."
+generatedocs: ## Format example Terraform configurations and generate plugin documentation.
 	@echo "==> Formatting examples and generating docs..."
 	terraform fmt -recursive ./examples/
 	go run github.com/hashicorp/terraform-plugin-docs/cmd/tfplugindocs generate
 
 .PHONY: help
 help: ## Show this help message
 	@grep -hE '^[a-zA-Z0-9._-]+:.*?##' $(MAKEFILE_LIST) | \
-	awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-24s\033[0m %s\n", $$1, $$2}' | \
+	awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-16s\033[0m %s\n", $$1, $$2}' | \
 	sort

README.md

@@ -42,14 +42,15 @@ Run `make test`
 
 Acceptance tests apply test terraform configurations to a running AAP instance and make changes to resources in that instance, use with caution!
 
-To run acceptance tests locally, you will need a running instance of Ansible Automation Platform (AAP). You can either use an existing instance or deploy a local test environment using any supported method (e.g., containerized or VM-based deployment from official Red Hat resources).
+To run acceptance tests locally, start a local AAP instance following the [docker-compose instructions for local AWX development](https://github.com/ansible/awx/blob/devel/tools/docker-compose/README.md). Create an admin user for the AAP instance and save the credentials to these environment variables:
 
 Create an admin user for the AAP instance and set the following environment variables:
 
 ```bash
 export AAP_USERNAME=<your admin username>
 export AAP_PASSWORD=<your admin password>
-export AAP_HOST="http://localhost:9080" # if using aap-dev (Note: Subject to change)
+export AAP_INSECURE_SKIP_VERIFY=true 
+export AAP_HOST=<your aap instance host url> # "http://localhost:9080" or "https://localhost:8043" 
 ```
 
 In order to run the acceptance tests for the job resource, you must have a working job template already in your AAP instance. The job template must be set to require an inventory on launch. Export the id of this job template:

internal/provider/client.go

@@ -116,9 +116,6 @@ func (c *AAPClient) computeURLPath(path string) string {
 
 func (c *AAPClient) doRequest(method string, path string, data io.Reader) (*http.Response, []byte, error) {
 	ctx := context.Background()
-	if c.httpClient == nil {
-		return nil, nil, fmt.Errorf("internal error: httpClient is nil")
-	}
 	req, err := http.NewRequestWithContext(ctx, method, c.computeURLPath(path), data)
 	if err != nil {
 		return nil, []byte{}, err
@@ -153,28 +150,8 @@ func (c *AAPClient) Create(path string, data io.Reader) ([]byte, diag.Diagnostic
 
 // Get sends a GET request to the provided path, checks for errors, and returns the response body with any errors as diagnostics.
 func (c *AAPClient) GetWithStatus(path string) ([]byte, diag.Diagnostics, int) {
-	if c == nil {
-		return nil, diag.Diagnostics{diag.NewErrorDiagnostic("AAPClient is nil", "Client was not initialized properly")}, 0
-	}
-	if path == "" {
-		return nil, diag.Diagnostics{diag.NewErrorDiagnostic("Path is empty", "Path cannot be empty")}, 0
-	}
-	if c.httpClient == nil {
-		return nil, diag.Diagnostics{diag.NewErrorDiagnostic("HttpClient is nil", "HttpClient was not initialized properly")}, 0
-	}
 	getResponse, body, err := c.doRequest("GET", path, nil)
 	diags := ValidateResponse(getResponse, body, err, []int{http.StatusOK})
-	if diags.HasError() {
-		return nil, diags, getResponse.StatusCode
-	}
-	if getResponse == nil {
-		diags.AddError("Get response is nil", "The response from the server is nil")
-		return nil, diags, 0
-	}
-	if body == nil {
-		diags.AddError("Get response body is nil", "The response body from the server is nil")
-		return nil, diags, 0
-	}
 	return body, diags, getResponse.StatusCode
 }
 

internal/provider/inventory_resource_test.go

@@ -188,7 +188,7 @@ func TestAccInventoryResource(t *testing.T) {
 				Check: resource.ComposeAggregateTestCheckFunc(
 					testAccCheckInventoryResourceExists(resourceNameInventory, &inventory),
 					testAccCheckInventoryResourceValues(&inventory, randomName, "", ""),
-					checkBasicInventoryAttributes(t, resourceNameInventory, randomName),
+					checkBasicInventoryAttributes(t, resourceNameInventory, randomName, "1", "Default"),
 				),
 			},
 			// Update and Read testing
@@ -197,7 +197,7 @@ func TestAccInventoryResource(t *testing.T) {
 				Check: resource.ComposeAggregateTestCheckFunc(
 					testAccCheckInventoryResourceExists(resourceNameInventory, &inventory),
 					testAccCheckInventoryResourceValues(&inventory, updatedName, updatedDescription, updatedVariables),
-					checkBasicInventoryAttributes(t, resourceNameInventory, updatedName),
+					checkBasicInventoryAttributes(t, resourceNameInventory, updatedName, "1", "Default"),
 					resource.TestCheckResourceAttr(resourceNameInventory, "description", updatedDescription),
 					resource.TestCheckResourceAttr(resourceNameInventory, "variables", updatedVariables),
 				),

internal/provider/utils_test.go

@@ -62,12 +62,12 @@ func checkBasicGroupAttributes(t *testing.T, name, expectedName string) resource
 	)
 }
 
-func checkBasicInventoryAttributes(t *testing.T, name, expectedName string) resource.TestCheckFunc {
+func checkBasicInventoryAttributes(t *testing.T, name, expectedName string, expectedOrgId string, expectedOrgName string) resource.TestCheckFunc {
 	t.Helper()
 	return resource.ComposeAggregateTestCheckFunc(
 		resource.TestCheckResourceAttr(name, "name", expectedName),
-		resource.TestCheckResourceAttr(name, "organization", "1"),
-		resource.TestCheckResourceAttr(name, "organization_name", "Default"),
+		resource.TestCheckResourceAttr(name, "organization", expectedOrgId),
+		resource.TestCheckResourceAttr(name, "organization_name", expectedOrgName),
 		resource.TestMatchResourceAttr(name, "url", reInventoryURLPattern),
 		resource.TestCheckResourceAttr(name, "named_url", fmt.Sprintf("/api/controller/v2/inventories/%s++%s/", expectedName, "Default")),
 		resource.TestCheckResourceAttrSet(name, "id"),

makefiles/golangci.mk

@@ -20,7 +20,7 @@ lint: lint-tools ## Run static analysis via golangci-lint
 	@echo "==> Checking source code against linters..."
 	$(GOLANGCI_LINT) run -v ./...
 
-gofmt: ## Format Go source code in 'internal/provider' using gofmt."
+gofmt: ## Format Go source code in 'internal/provider' using gofmt.
 	@echo "==> Format code using gofmt..."
 	gofmt -s -w internal/provider