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

Add azuredevops_serviceendpoint_aws_terraform resource #767

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

jwoffindin
Copy link

@jwoffindin jwoffindin commented May 12, 2023

Why:

Microsoft Labs publish an extension "Terraform Extension for Azure DevOps" https://github.com/microsoft/azure-pipelines-terraform.

It provides high-level wapper around terraform for Azure DevOps pipelines and adds 3 custom service endpoints (AWS, GCP and Azure) that can be used from Azure Pipeline.

I want to use this library with Azure DevOps so I can use Terraform to manage my Azure DevOps pipelines that deploy into AWS.

Unfortunately this extension does not work with the standard AWS service endpoint. If you try to use standard aws endpoint, the pipeline fails with:

Job Terraform: Step  input backendServiceAWS expects a service connection of type AWSServiceEndpoint but the provided service connection XXX is of type aws.

So currently I need to create and manage these service endpoints manually, rather than using Terraform, which makes me sad :-/

This change addresses the need by:

  • Adding a new resource implementation azuredevops_serviceendpoint_aws_terraform. It is pretty much a rip-off of the existing aws endpoint provider with change to type and the parameters it accepts (only access key/secret and region).

  • Adding unit and integration tests. Again, a blatant rip-off of the existing aws endpoint tests.

  • Add webpage documentation for the new provider.

Note this change only adds support for AWS backend. It does not add support for the GCP/Azure backends - I don't have enough experience to validate a provider for these environments.

Refs: #675

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?

No changes to existing functionality. It just adds a new resource. Note, that while this endpoint is AWS related, it's
not the same as the existing azuredevops_serviceendpoint_aws provider, confusingly enough.

Issue Number: #675

This existing issue was raised for GCP and AWS, although the original author was more interested in GCP support.

This change should add the requested support for AWS.

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?

This is a new feature, so no existing logs.

Other information

Here's an example from my testing:

resource "azuredevops_serviceendpoint_aws_terraform" "endpoint" {
  project_id            = var.project_id
  service_endpoint_name = "example-connection"
  access_key_id         = aws_iam_access_key.pipeline-user.id
  secret_access_key     = aws_iam_access_key.pipeline-user.secret
  region                = "us-east-1"
  description           = "Managed by Terraform"
}

and attached a screenshot of service endpoint resource it creates :

image

@jwoffindin jwoffindin changed the title Adds azuredevops_serviceendpoint_aws_terraform resource Add azuredevops_serviceendpoint_aws_terraform resource May 12, 2023
@jwoffindin
Copy link
Author

@microsoft-github-policy-service agree

@jwoffindin jwoffindin marked this pull request as ready for review May 12, 2023 07:43
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.

@jwoffindin Acctest failed:

=== PAUSE TestAccServiceEndpointAwsTerraform_Basic
=== CONT  TestAccServiceEndpointAwsTerraform_Basic
    resource_serviceendpoint_aws_terraform_test.go:17: Step 1/1 error: Check failed: Check 6/7 error: azuredevops_serviceendpoint_aws_terraform.test: Attribute 'region' expected "", got "us-east-1"
--- FAIL: TestAccServiceEndpointAwsTerraform_Basic (58.08s)
FAIL
FAIL   

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.

@jwoffindin Acctest failed:

=== PAUSE TestAccServiceEndpointAwsTerraform_Basic
=== CONT  TestAccServiceEndpointAwsTerraform_Basic
    resource_serviceendpoint_aws_terraform_test.go:17: Step 1/1 error: Check failed: Check 6/7 error: azuredevops_serviceendpoint_aws_terraform.test: Attribute 'region' expected "", got "us-east-1"
--- FAIL: TestAccServiceEndpointAwsTerraform_Basic (58.08s)
FAIL
FAIL   

@xuzhang3
Copy link
Collaborator

Please update the latest code from upstream, we recently upgrade the API from v6 to v7.

jwoffindin and others added 2 commits May 24, 2023 19:49
Why:

Microsoft Labs publish an extension "Terraform Extension for Azure DevOps"
<https://github.com/microsoft/azure-pipelines-terraform>.

It provides high-level wapper around terraform for Azure DevOps pipelines
and adds 3 custom service endpoints (AWS, GCP and Azure) that can be
used from Azure Pipeline.

I want to use this library with Azure DevOps to deploy into my AWS account.

Unfortunately this extension does _not_ work with the standard AWS service
endpoint. If you try to use standard `aws` endpoint, the pipeline fails with:

> Job Terraform: Step  input backendServiceAWS expects a service connection of type AWSServiceEndpoint but the provided service connection XXX is of type aws.

This change addresses the need by:

* Adding a new resource implementation `azuredevops_serviceendpoint_aws_terraform`.
  It is pretty much a rip-off of the existing aws endpoint provider with change
  to type and the parameters it accepts (only access key/secret and region).

* Adding unit and integration tests. Again, a blatant rip-off of the existing
  aws endpoint tests.

* Add webpage documentation for the new provide.

Note this change only adds support for AWS backend. It does not add support for
the GCP/Azure backends - I don't have enough experience to validate a provider
for these environments.

Refs: microsoft#675
Why:

Microsoft Labs publish an extension "Terraform Extension for Azure DevOps"
<https://github.com/microsoft/azure-pipelines-terraform>.

It provides high-level wapper around terraform for Azure DevOps pipelines
and adds 3 custom service endpoints (AWS, GCP and Azure) that can be
used from Azure Pipeline.

I want to use this library with Azure DevOps to deploy into my AWS account.

Unfortunately this extension does _not_ work with the standard AWS service
endpoint. If you try to use standard `aws` endpoint, the pipeline fails with:

> Job Terraform: Step  input backendServiceAWS expects a service connection of type AWSServiceEndpoint but the provided service connection XXX is of type aws.

This change addresses the need by:

* Adding a new resource implementation `azuredevops_serviceendpoint_aws_terraform`.
  It is pretty much a rip-off of the existing aws endpoint provider with change
  to type and the parameters it accepts (only access key/secret and region).

* Adding unit and integration tests. Again, a blatant rip-off of the existing
  aws endpoint tests.

* Add webpage documentation for the new provide.

Note this change only adds support for AWS backend. It does not add support for
the GCP/Azure backends - I don't have enough experience to validate a provider
for these environments.

Refs: microsoft#675
Why:

* Service connector expects all fields (access key/secret/region) to be present.
feedback from @xuzhang3 "Hash function has been deprecated and can be removed."
@jwoffindin jwoffindin requested a review from xuzhang3 May 24, 2023 08:43
sessionToken := "foobar"
rta := "rta"
rsn := "rsn"
externalId := "external_id"
Copy link
Collaborator

Choose a reason for hiding this comment

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

external_id not in the schema, can be deleted

resource.TestCheckResourceAttr(tfSvcEpNode, "secret_access_key", "secretkey"),
resource.TestCheckResourceAttr(tfSvcEpNode, "description", description),
resource.TestCheckResourceAttr(tfSvcEpNode, "region", "us-east-1"),
resource.TestCheckResourceAttr(tfSvcEpNode, "external_id", externalId),
Copy link
Collaborator

Choose a reason for hiding this comment

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

external_id not in the schema, can be deleted

region = "us-east-1"
service_endpoint_name = "%s"
description = "%s"
external_id = "%s"
Copy link
Collaborator

Choose a reason for hiding this comment

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

external_id not in the schema, can be deleted

return fmt.Sprintf("%s\n%s", projectResource, serviceEndpointResource)
}

func hclSvcEndpointAwsTerraformResourceComplete(projectName string, serviceEndpointName string, description string, sessionToken string, rta string, rsn string, externalId string) string {
Copy link
Collaborator

Choose a reason for hiding this comment

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

essionToken, rta s, rsn

Suggested change
func hclSvcEndpointAwsTerraformResourceComplete(projectName string, serviceEndpointName string, description string, sessionToken string, rta string, rsn string, externalId string) string {
func hclSvcEndpointAwsTerraformResourceComplete(projectName, serviceEndpointName, description string) string {

DefaultFunc: schema.EnvDefaultFunc("AZDO_TF_AWS_SERVICE_CONNECTION_SECRET_ACCESS_KEY", nil),
Description: "The AWS secret access key for signing programmatic requests.",
Sensitive: true,
DiffSuppressFunc: tfhelper.DiffFuncSuppressSecretChanged,
Copy link
Collaborator

Choose a reason for hiding this comment

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

DiffSuppressFunc can delete as it is used by the deprecate hash fucntion.

Comment on lines +61 to +65
tfhelper.HelpFlattenSecret(d, "secret_access_key")

d.Set("access_key_id", (*serviceEndpoint.Authorization.Parameters)["username"])
d.Set("secret_access_key", (*serviceEndpoint.Authorization.Parameters)["password"])
d.Set("region", (*serviceEndpoint.Authorization.Parameters)["region"])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sensitive data will always be returned as empty string. Leave unchange or get the value from the HCL.

Suggested change
tfhelper.HelpFlattenSecret(d, "secret_access_key")
d.Set("access_key_id", (*serviceEndpoint.Authorization.Parameters)["username"])
d.Set("secret_access_key", (*serviceEndpoint.Authorization.Parameters)["password"])
d.Set("region", (*serviceEndpoint.Authorization.Parameters)["region"])
tfhelper.HelpFlattenSecret(d, "secret_access_key")
d.Set("secret_access_key", d.Get("secret_access_key").(string))
d.Set("access_key_id", (*serviceEndpoint.Authorization.Parameters)["username"])
d.Set("region", (*serviceEndpoint.Authorization.Parameters)["region"])

CheckDestroy: testutils.CheckServiceEndpointDestroyed(resourceType),
Steps: []resource.TestStep{
{
Config: hclSvcEndpointAwsTerraformResourceComplete(projectName, serviceEndpointName, description, sessionToken, rta, rsn, externalId),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Config: hclSvcEndpointAwsTerraformResourceComplete(projectName, serviceEndpointName, description, sessionToken, rta, rsn, externalId),
Config: hclSvcEndpointAwsTerraformResourceComplete(projectName, serviceEndpointName, description),

Comment on lines +41 to +43
sessionToken := "foobar"
rta := "rta"
rsn := "rsn"
Copy link
Collaborator

Choose a reason for hiding this comment

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

can be remove as not used

Comment on lines +53 to +54
* [azure-pipelines-terraform](https://github.com/microsoft/azure-pipelines-terraform)
* [Azure DevOps Service REST API 6.0 - Agent Pools](https://docs.microsoft.com/en-us/rest/api/azure/devops/serviceendpoint/endpoints?view=azure-devops-rest-6.0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* [azure-pipelines-terraform](https://github.com/microsoft/azure-pipelines-terraform)
* [Azure DevOps Service REST API 6.0 - Agent Pools](https://docs.microsoft.com/en-us/rest/api/azure/devops/serviceendpoint/endpoints?view=azure-devops-rest-6.0)
* [Azure DevOps Service REST API 7.0 - Service connections](https://docs.microsoft.com/en-us/rest/api/azure/devops/serviceendpoint/endpoints?view=azure-devops-rest-7.0)

* `access_key_id` - (Required) The AWS access key ID for signing programmatic requests.
* `secret_access_key` - (Required) The AWS secret access key for signing programmatic requests.
* `region` - (Required) The AWS region to send requests to.
* `description` - (Optional) The Service Endpoint description. Defaults to `Managed by Terraform`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* `description` - (Optional) The Service Endpoint description. Defaults to `Managed by Terraform`.
---
* `description` - (Optional) The Service Endpoint description. Defaults to `Managed by Terraform`.

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.

@jwoffindin A few changes requested, others looks good to me.

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.

2 participants