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

feat(resource_git_repository): add support for importing private repos #402

Merged
merged 5 commits into from
Jun 29, 2021

Conversation

dmccaffery
Copy link
Contributor

@dmccaffery dmccaffery commented Jun 11, 2021

feat(resource_git_repository): add support for importing private repos

  • add support for specifying a service_connection_id for importing
    private repositories using the serviceendpoint_generic_git resource
  • add acceptance test that imports a repository from azure devops via
    the serviceendpoint_generic_git resource
  • fix an issue where errors in the import / initialization process could
    leave behind the uninitialized repository without being recorded in
    state

Closes: #236
Supersedes: #181

feat: add generic git service endpoint resource

  • add serviceendpoint_generic_git resource to enable creation of a
    completely authentication service endpoint for any git repository url
  • add acceptance tests for the resource
  • add documentation for the resource

feat: add generic service endpoint resource

  • add serviceendpoint_generic resource to enable creation of a
    completely generic authentication service endpoint for any server
  • add acceptance tests for the resource
  • add documentation for the resource

fix: add missing build tags on acceptance tests

  • add build tags on resource_repositorypolicy_author_email_patterns_test
  • add build tags on resource_serviceendpoint_azuredevops_test
  • add build tags on resource_serviceendpoint_ssh_test

All Submissions:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • My code follows the code style of this project.
  • I ran lint checks locally prior to submission.
  • Have you checked to ensure there aren't other open PRs for the same update/change?

What about the current behavior has changed?

Issue Number: #236

Does this introduce a change to go.mod, go.sum or vendor/?

  • Yes
  • No

Does this introduce a breaking change?

  • Yes
  • No

Any relevant logs, error output, etc?

(If it’s long, please paste to https://ghostbin.com/ and insert the link here.)

Other information

@xuzhang3
Copy link
Collaborator

xuzhang3 commented Jun 15, 2021

Hi @dmccaffery Thanks for PR.
The import authorization is based on service connection Generic service.
I would suggest replace the username/password with service endpoint ID as the service request parameter asked

  • Git repository resource can be decoupled from service endpoint resource
  • Service endpoint can be reused in other resource.
  • The new created service connection need to be deleted after import finished which will made the Git Repository resource more complex.

There is a PR #181 working on generic git service endpoint, but no update for a long time. @paulbehrisch any update on PR #181 ?

@dmccaffery
Copy link
Contributor Author

Hi @dmccaffery Thanks for PR.
The import authorization is based on service connection Generic service.
I would suggest replace the username/password with service endpoint ID as the service request parameter asked

  • Git repository resource can be decoupled from service endpoint resource
  • Service endpoint can be reused in other resource.
  • The new created service connection need to be deleted after import finished which will made the Git Repository resource more complex.

There is a PR #181 working on generic git service endpoint, but no update for a long time. @paulbehrisch any update on PR #181 ?

I'm happy to implement this on top of an existing service connection once it is available. Re: deleting the service connection afterwards, there is a flag sent to the import request that causes the server to delete the service connection automatically after the import completes, so this doesn't need to be handled by us in terraform (other than setting the flag). We, of course, would not want to set this flag if passing in an existing service connection id instead of creating one within this resource.

It should be noted that I created this PR by reverse engineering the network traffic performed when doing an import via the UI, which always creates a new randomly named service connection and then passes in DeleteServiceEndpointAfterImportIsDone flag:

https://github.com/microsoft/terraform-provider-azuredevops/pull/402/files#diff-1c6ad48d90ac6ce910958242b9bcc943c4baea679f3028c62110654a227a01dbR217

We can put this PR on hold until the generic service connection is available. I can also look at taking over #181 if the original author and/or maintainers agree.

@dmccaffery
Copy link
Contributor Author

@xuzhang3: I took the liberty to reimplementing this on top of a new serviceendpoint_generic_git resource as described in #118. I also added acceptance tests for everything and verified that an import of a private repository via the service connection works as expected in a real-world scenario.

Let me know if there is anything else I can do!

Copy link
Collaborator

@xuzhang3 xuzhang3 left a comment

Choose a reason for hiding this comment

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

@dmccaffery Thanks for your PR, a few changes requested.

  1. azuredevops_serviceendpoint_generic and azuredevops_serviceendpoint_generic_git document should indicate that user can use username/password or username/personal access token to create the service. Especially for GitHub user with 2-factor-auth enabled, they should create the service connection with username/personal access token not username/password, otherwise the authorization will failed.
  2. azuredevops_git_repository can remove the username/password now, the import authorization can use the service connection.

* add build tags on `resource_repositorypolicy_author_email_patterns_test`
* add build tags on `resource_serviceendpoint_azuredevops_test`
* add build tags on `resource_serviceendpoint_ssh_test`
* add `serviceendpoint_generic` resource to enable creation of a
  completely generic authentication service endpoint for any server
* add acceptance tests for the resource
* add documentation for the resource
@dmccaffery
Copy link
Contributor Author

@xuzhang3 all requested changes have been made; also rebased on top of master.

* add `serviceendpoint_generic_git` resource to enable creation of a
  completely authentication service endpoint for any git repository url
* add acceptance tests for the resource
* add documentation for the resource
…sitories

* add support for specifying a `service_connection_id` for importing
  private repositories using the `serviceendpoint_generic_git` resource
* add acceptance test that imports a repository from azure devops via
  the `serviceendpoint_generic_git` resource
* fix an issue where errors in the import / initialization process could
  leave behind the uninitialized repository without being recorded in
  state

Closes: microsoft#236
Supersedes: microsoft#181
Copy link
Collaborator

@xuzhang3 xuzhang3 left a comment

Choose a reason for hiding this comment

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

serviceendpoint_generic.html.markdown and serviceendpoint_generic_git.html.markdown document link need to be add to azuredevops.erb

@xuzhang3
Copy link
Collaborator

@dmccaffery I have add the document link serviceendpoint_generic.html.markdown and serviceendpoint_generic_git.html.markdown document link to azuredevops.erb

@dmccaffery
Copy link
Contributor Author

@dmccaffery I have add the document link serviceendpoint_generic.html.markdown and serviceendpoint_generic_git.html.markdown document link to azuredevops.erb

Thank you! I've been super busy (moving home) and haven't had a chance to circle back on this. Much appreciated!

@xuzhang3
Copy link
Collaborator

Acceptance test result:

Git Repository

==> Checking that code complies with gofmt requirements...
==> Sourcing .env file if avaliable
if [ -f .env ]; then set -o allexport; . ./.env; set +o allexport; fi; \
        TF_ACC=1 go test -tags "all" $(go list ./azuredevops/internal/acceptancetests |grep -v 'vendor') -v -run=TestAccGitRepo -timeout 120m
=== RUN   TestAccGitRepository_DataSource
=== PAUSE TestAccGitRepository_DataSource
=== RUN   TestAccGitRepo_CreateAndUpdate
=== PAUSE TestAccGitRepo_CreateAndUpdate
=== RUN   TestAccGitRepo_Create_IncorrectInitialization
=== PAUSE TestAccGitRepo_Create_IncorrectInitialization
=== RUN   TestAccGitRepo_Create_Import
=== PAUSE TestAccGitRepo_Create_Import
=== RUN   TestAccGitRepo_RepoInitialization_Clean
=== PAUSE TestAccGitRepo_RepoInitialization_Clean
=== RUN   TestAccGitRepo_RepoInitialization_Uninitialized
=== PAUSE TestAccGitRepo_RepoInitialization_Uninitialized
=== RUN   TestAccGitRepo_RepoFork_BranchNotEmpty
=== PAUSE TestAccGitRepo_RepoFork_BranchNotEmpty
=== RUN   TestAccGitRepo_PrivateImport_BranchNotEmpty
=== PAUSE TestAccGitRepo_PrivateImport_BranchNotEmpty
=== CONT  TestAccGitRepository_DataSource
=== CONT  TestAccGitRepo_RepoInitialization_Uninitialized
=== CONT  TestAccGitRepo_Create_Import
=== CONT  TestAccGitRepo_PrivateImport_BranchNotEmpty
=== CONT  TestAccGitRepo_RepoInitialization_Clean
=== CONT  TestAccGitRepo_Create_IncorrectInitialization
=== CONT  TestAccGitRepo_RepoFork_BranchNotEmpty
=== CONT  TestAccGitRepo_CreateAndUpdate
--- PASS: TestAccGitRepo_Create_IncorrectInitialization (0.14s)
--- PASS: TestAccGitRepo_CreateAndUpdate (80.69s)
--- PASS: TestAccGitRepo_Create_Import (112.92s)
--- PASS: TestAccGitRepo_RepoInitialization_Clean (123.01s)
--- PASS: TestAccGitRepository_DataSource (184.16s)
--- PASS: TestAccGitRepo_RepoFork_BranchNotEmpty (204.82s)
--- PASS: TestAccGitRepo_RepoInitialization_Uninitialized (219.02s)
--- PASS: TestAccGitRepo_PrivateImport_BranchNotEmpty (236.31s)
PASS
ok      github.com/microsoft/terraform-provider-azuredevops/azuredevops/internal/acceptancetests        237.321s

Generic service connection

==> Checking that code complies with gofmt requirements...
==> Sourcing .env file if avaliable
if [ -f .env ]; then set -o allexport; . ./.env; set +o allexport; fi; \
        TF_ACC=1 go test -tags "all" $(go list ./azuredevops/internal/acceptancetests |grep -v 'vendor') -v -run=TestAccServiceEndpointGeneric -timeout 120m
=== RUN   TestAccServiceEndpointGenericGit_basic
=== PAUSE TestAccServiceEndpointGenericGit_basic
=== RUN   TestAccServiceEndpointGenericGit_complete
=== PAUSE TestAccServiceEndpointGenericGit_complete
=== RUN   TestAccServiceEndpointGenericGit_update
=== PAUSE TestAccServiceEndpointGenericGit_update
=== RUN   TestAccServiceEndpointGenericGit_RequiresImportErrorStep
=== PAUSE TestAccServiceEndpointGenericGit_RequiresImportErrorStep
=== RUN   TestAccServiceEndpointGeneric_basic
=== PAUSE TestAccServiceEndpointGeneric_basic
=== RUN   TestAccServiceEndpointGeneric_complete
=== PAUSE TestAccServiceEndpointGeneric_complete
=== RUN   TestAccServiceEndpointGeneric_update
=== PAUSE TestAccServiceEndpointGeneric_update
=== RUN   TestAccServiceEndpointGeneric_RequiresImportErrorStep
=== PAUSE TestAccServiceEndpointGeneric_RequiresImportErrorStep
=== CONT  TestAccServiceEndpointGenericGit_basic
=== CONT  TestAccServiceEndpointGeneric_complete
=== CONT  TestAccServiceEndpointGenericGit_RequiresImportErrorStep
=== CONT  TestAccServiceEndpointGeneric_basic
=== CONT  TestAccServiceEndpointGenericGit_update
=== CONT  TestAccServiceEndpointGenericGit_complete
=== CONT  TestAccServiceEndpointGeneric_RequiresImportErrorStep
=== CONT  TestAccServiceEndpointGeneric_update
--- PASS: TestAccServiceEndpointGeneric_basic (122.44s)
--- PASS: TestAccServiceEndpointGenericGit_complete (145.90s)
--- PASS: TestAccServiceEndpointGeneric_update (172.42s)
--- PASS: TestAccServiceEndpointGeneric_complete (175.42s)
--- PASS: TestAccServiceEndpointGenericGit_RequiresImportErrorStep (184.66s)
--- PASS: TestAccServiceEndpointGenericGit_basic (193.94s)
--- PASS: TestAccServiceEndpointGeneric_RequiresImportErrorStep (199.21s)
--- PASS: TestAccServiceEndpointGenericGit_update (206.17s)
PASS
ok      github.com/microsoft/terraform-provider-azuredevops/azuredevops/internal/acceptancetests        207.033s

Generic Git service connection

==> Checking that code complies with gofmt requirements...
==> Sourcing .env file if avaliable
if [ -f .env ]; then set -o allexport; . ./.env; set +o allexport; fi; \
        TF_ACC=1 go test -tags "all" $(go list ./azuredevops/internal/acceptancetests |grep -v 'vendor') -v -run=TestAccServiceEndpointGenericGit -timeout 120m
=== RUN   TestAccServiceEndpointGenericGit_basic
=== PAUSE TestAccServiceEndpointGenericGit_basic
=== RUN   TestAccServiceEndpointGenericGit_complete
=== PAUSE TestAccServiceEndpointGenericGit_complete
=== RUN   TestAccServiceEndpointGenericGit_update
=== PAUSE TestAccServiceEndpointGenericGit_update
=== RUN   TestAccServiceEndpointGenericGit_RequiresImportErrorStep
=== PAUSE TestAccServiceEndpointGenericGit_RequiresImportErrorStep
=== CONT  TestAccServiceEndpointGenericGit_basic
=== CONT  TestAccServiceEndpointGenericGit_RequiresImportErrorStep
=== CONT  TestAccServiceEndpointGenericGit_complete
=== CONT  TestAccServiceEndpointGenericGit_update
--- PASS: TestAccServiceEndpointGenericGit_complete (82.92s)
--- PASS: TestAccServiceEndpointGenericGit_RequiresImportErrorStep (94.03s)
--- PASS: TestAccServiceEndpointGenericGit_basic (147.33s)
--- PASS: TestAccServiceEndpointGenericGit_update (172.29s)
PASS
ok      github.com/microsoft/terraform-provider-azuredevops/azuredevops/internal/acceptancetests        173.143s

@xuzhang3
Copy link
Collaborator

LGTM

@xuzhang3 xuzhang3 merged commit ad6cf4d into microsoft:master Jun 29, 2021
@dmccaffery dmccaffery deleted the feat/private-repositories branch June 29, 2021 10:11
@xuzhang3 xuzhang3 linked an issue Jul 20, 2021 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support import private repository Support Other Git service connections
2 participants