Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix boundary_worker auth token overwritten during creation & ForceNew on auth token change #461

Merged
merged 2 commits into from
Dec 13, 2023

Conversation

drewmullen
Copy link
Contributor

@drewmullen drewmullen commented Sep 21, 2023

closes: #459

How can i get the token the local deployment is expecting for testing a worker deployment? I tried without setting a token (error'd) and i tried using the token that is set as a const in the worker_test.go file. Both failed

setup db

$ make test-database-up                                                                          
Using image:                       docker.io/hashicorpboundary/postgres:11-alpine
Additional postgres configuration: 
Using volume:                      /Users/dm/go/pkg/mod /github.com/hashicorp/[email protected]/internal/db/schema/migrations:/migrations
...
Status: Downloaded newer image for hashicorpboundary/postgres:11-alpine
Test database available at:        127.0.0.1:5432
For database logs run:
    docker logs boundary-sql-tests

Set token from const value in worker_test:

$ export BOUNDARY_TF_PROVIDER_TEST_WORKER_LED_TOKEN='GzusqckarbczHoLGQ4UA25uSRGy7e7hUk4Qgz8T3NmSZF4oXBc1dKMRyH7Mr6W5u2QWggkZi1wsnfYMufb5LZJJnxEdywUxpE8RgAbVTahMhJm8oeH3nagrAKfWJkrTt8QuoqiupcTrFmJtDPZ5WBtqKSXiW3jdG8esCt7ZNKVaKV1Hq6MBXUf7duj9KfAy4Y3B31jDTzoF1uVnK1AsaLkEhgbHbuAH33L2KG5ivo7YeFeE6PknNoVavPSRSkoEpcXSfjvoPMAz9ttpNvH7jGWPwLti8r48NcVj41ftXWg'

run updated test:

$ make testacc TESTARGS='-run=TestWorkerWorkerLed'
TF_ACC=1 go test ./... -v -run=TestWorkerWorkerLed -timeout 120m
?       github.com/hashicorp/terraform-provider-boundary        [no test files]
?       github.com/hashicorp/terraform-provider-boundary/plugins/kms    [no test files]
=== RUN   TestWorkerWorkerLed
    worker_test.go:71: Step 1/5 error: Error running apply: exit status 1
        
        Error: error creating worker: {"kind":"Internal", "message":"workers.(Service).CreateWorkerLed: workers.(Service).createInRepo: unable to create worker: server.CreateWorker: db.DoTx: server.CreateWorker: unable to authorize node: unknown: error #0: (nodeenrollment.registration.AuthorizeNode) error during fetch request validation: (nodeenrollment.registration.validateFetchRequest) validity period is before current time"}
        
          with boundary_worker.worker_led,
          on terraform_plugin_test.tf line 9, in resource "boundary_worker" "worker_led":
           9:   resource "boundary_worker" "worker_led" {
        
--- FAIL: TestWorkerWorkerLed (8.31s)
FAIL
FAIL    github.com/hashicorp/terraform-provider-boundary/internal/provider      8.635s
FAIL
make: *** [testacc] Error 1

@drewmullen drewmullen changed the title Fix auth token overwrite in create & ForceNew on auth token change Fix boundary_worker auth token overwrite in create & ForceNew on auth token change Sep 21, 2023
@drewmullen drewmullen changed the title Fix boundary_worker auth token overwrite in create & ForceNew on auth token change Fix boundary_worker auth token overwritten during creation & ForceNew on auth token change Sep 29, 2023
@louisruch
Copy link
Contributor

@drewmullen if you merge main into this or rebase on main we can have it run tests and then merge it in as well

@drewmullen
Copy link
Contributor Author

Looks like the tests i changed were skipped:

=== RUN   TestWorkerWorkerLed
    worker_test.go:63: Not running worker led activation test without worker led token present
--- SKIP: TestWorkerWorkerLed (0.00s)

I was unable to determine how to run these tests locally. Not sure if there is a way - happy to try it locally if there is!

@louisruch
Copy link
Contributor

louisruch commented Oct 18, 2023

@grantorchard wrote these tests originally he may be able to help, but that being said these changes LGTM

@drewmullen
Copy link
Contributor Author

when i run this locally i get the exact same error as CI:

$ make testacc TESTARGS='-run=TestWorkerWorkerLed'
TF_ACC=1 go test ./... -v -run=TestWorkerWorkerLed -timeout 120m
?       github.com/hashicorp/terraform-provider-boundary        [no test files]
?       github.com/hashicorp/terraform-provider-boundary/plugins/kms    [no test files]
=== RUN   TestWorkerWorkerLed
    worker_test.go:63: Not running worker led activation test without worker led token present
--- SKIP: TestWorkerWorkerLed (0.00s)
PASS
ok      github.com/hashicorp/terraform-provider-boundary/internal/provider      0.662s

I tried setting the token to the same set in the const but that didnt seem to resolve (see issue description). @grantorchard lmk if you have any advice

@psekar psekar requested review from talanknight and elimt November 15, 2023 17:54
@elimt
Copy link
Member

elimt commented Nov 20, 2023

The changes LGTM. worker_generated_auth_token is input only so it won't be returned

For the test, I took a look at it.

  1. You need a valid token to successfully run the test locally. So you need to start a self managed worker and grab the token
  2. Set BOUNDARY_TF_PROVIDER_TEST_WORKER_LED_TOKEN with the token
  3. TestWorkerWorkerLed test needs to use the token in for the terraform config. Something like:
workerLedCreate = `
	resource "boundary_worker" "worker_led" {
		scope_id = "global"
		name = "%s"
		description = "%s"
		worker_generated_auth_token = "%s"
	}`

func TestWorkerWorkerLed(t *testing.T) {
	token := os.Getenv("BOUNDARY_TF_PROVIDER_TEST_WORKER_LED_TOKEN")
	if token == "" {
		t.Skip("Not running worker led activation test without worker led token present")
	}

	workerLedCreateWithToken := fmt.Sprintf(workerLedCreate, workerName, workerDesc, token)

	tc := controller.NewTestController(t, tcConfig...)
	defer tc.Shutdown()
	url := tc.ApiAddrs()[0]

	var provider *schema.Provider
	resource.Test(t, resource.TestCase{
		IsUnitTest:        true,
		ProviderFactories: providerFactories(&provider),
		CheckDestroy:      testAccCheckworkerResourceDestroy(t, provider),
		Steps: []resource.TestStep{
			{
				// create
				Config: testConfig(url, workerLedCreateWithToken),
				Check: resource.ComposeTestCheckFunc(
					testAccCheckworkerResourceExists(provider, "boundary_worker.worker_led"),
					resource.TestCheckResourceAttr("boundary_worker.worker_led", "description", workerDesc),
					resource.TestCheckResourceAttr("boundary_worker.worker_led", "name", workerName),
				),
			},
			importStep("boundary_worker.worker_led", workerGeneratedAuthToken),
make testacc TESTARGS='-run=TestWorkerWorkerLed'
TF_ACC=1 go test ./... -v -run=TestWorkerWorkerLed -timeout 120m
?       github.com/hashicorp/terraform-provider-boundary        [no test files]
?       github.com/hashicorp/terraform-provider-boundary/plugins/kms    [no test files]
=== RUN   TestWorkerWorkerLed
--- PASS: TestWorkerWorkerLed (7.45s)
PASS
ok      github.com/hashicorp/terraform-provider-boundary/internal/provider      8.534s

I don't believe we can run this in CI yet but you can test it locally. Let me know if you have any clarifying questions

@drewmullen
Copy link
Contributor Author

Hi @elimt Thank you for the input. I looks like you were able to run the tests successfully! Nice. IMO this is good to merge at this point if yall are OK with it, @louisruch .

@drewmullen
Copy link
Contributor Author

@moduli any chance this can get merged or closed if not accepted? 🙏

@moduli
Copy link
Contributor

moduli commented Dec 13, 2023

@moduli any chance this can get merged or closed if not accepted? 🙏

Thanks for bringing it up. I need to wrap up a few things first, but I'll follow up/raise it to someone.

@psekar psekar merged commit f8b6987 into hashicorp:main Dec 13, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

boundary_worker arg worker_generated_auth_token should ForceNew and stored properly in state
5 participants