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

Adds Agent Pool to OAuth Client API #841

Merged
merged 16 commits into from
Feb 27, 2024

Conversation

roleesinhaHC
Copy link
Contributor

@roleesinhaHC roleesinhaHC commented Jan 29, 2024

Description

Currently, Private VCS support is enabled in TFC behind feature flag. This PR include changes to add API support for Private VCS by enabling creation of OAuth Client when AgentPoolID is set (as an optional param).

Testing plan

  1. Enable Private VCS for in a test org.
  2. Create an Agent Pool in the org.
  3. Launch an tfc-agent running locally.
  4. Execute the oauth_client creation API and pass the agent_pool_id as a param.

Output from tests

Including output from tests may require access to a TFE instance. Ignore this section if you have no environment to test against.

go-tfe % TFE_TOKEN= TFE_ADDRESS= OAUTH_CLIENT_GITHUB_TOKEN= go test -run TestOAuthClientsCreate_agentPool -v ./...
?       github.com/hashicorp/go-tfe/examples/backing_data       [no test files]
?       github.com/hashicorp/go-tfe/examples/configuration_versions     [no test files]
?       github.com/hashicorp/go-tfe/examples/organizations      [no test files]
?       github.com/hashicorp/go-tfe/examples/registry_modules   [no test files]
?       github.com/hashicorp/go-tfe/examples/run_errors [no test files]
?       github.com/hashicorp/go-tfe/examples/state_versions     [no test files]
?       github.com/hashicorp/go-tfe/examples/users      [no test files]
?       github.com/hashicorp/go-tfe/examples/workspaces [no test files]
?       github.com/hashicorp/go-tfe/mocks       [no test files]
=== RUN   TestOAuthClientsCreate_agentPool
=== RUN   TestOAuthClientsCreate_agentPool/with_valid_agent_pool_external_id
=== RUN   TestOAuthClientsCreate_agentPool/with_an_invalid_agent_pool
=== RUN   TestOAuthClientsCreate_agentPool/with_no_agents_connected
--- PASS: TestOAuthClientsCreate_agentPool (5.44s)
    --- PASS: TestOAuthClientsCreate_agentPool/with_valid_agent_pool_external_id (1.81s)
    --- PASS: TestOAuthClientsCreate_agentPool/with_an_invalid_agent_pool (0.43s)
    --- PASS: TestOAuthClientsCreate_agentPool/with_no_agents_connected (0.38s)
PASS
ok      github.com/hashicorp/go-tfe     5.911s
...

@roleesinhaHC roleesinhaHC changed the title Rolees/update params for oauth client create Add Agent Pool to oauth_client create API Jan 29, 2024
Copy link
Collaborator

@brandonc brandonc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

errors.go Outdated Show resolved Hide resolved
@roleesinhaHC roleesinhaHC marked this pull request as ready for review February 12, 2024 23:06
@roleesinhaHC roleesinhaHC requested a review from a team as a code owner February 12, 2024 23:06
@@ -95,6 +95,8 @@ type OAuthClient struct {
// Relations
Organization *Organization `jsonapi:"relation,organization"`
OAuthTokens []*OAuthToken `jsonapi:"relation,oauth-tokens"`
AgentPool *AgentPool `jsonapi:"relation,agent-pool"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears that you've also added the inverse relationship to the API (each agent pool has_many oauth-clients). What do you think about adding include_data = false to that new relationship in the API? Is there a good reason to serialize all of the oauth-clients associated with an agent pool?

Copy link
Contributor Author

@roleesinhaHC roleesinhaHC Feb 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, added those changes https://github.com/hashicorp/atlas/pull/18747

}

t.Run("with valid agent pool external id", func(t *testing.T) {
t.Skip()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments here would be appropriate

@roleesinhaHC roleesinhaHC changed the title Add Agent Pool to oauth_client create API Adds Agent Pool to OAuth Client API Feb 27, 2024
@uturunku1 uturunku1 self-requested a review February 27, 2024 21:21
Copy link
Collaborator

@uturunku1 uturunku1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for walking me through your changes over the zoom meeting!

@roleesinhaHC roleesinhaHC merged commit 45faff8 into main Feb 27, 2024
10 checks passed
@roleesinhaHC roleesinhaHC deleted the rolees/update_params_for_oauth_client_create branch February 27, 2024 21:28
Copy link

Reminder to the contributor that merged this PR: if your changes have added important functionality or fixed a relevant bug, open a follow-up PR to update CHANGELOG.md with a note on your changes.

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.

3 participants