New Glue Data sources - Glue Connection and Glue Data Catalog Encryption Settings #18802

danu165 · 2021-04-12T18:02:34Z

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

This PR adds new data sources for aws_glue_connection and aws_glue_data_catalog_encryption_settings.

Note: I was not able to test this due to odd behavior from make fmt. It modifies almost every file where it changes all instances of "%w" to "%s". I would appreciate it if someone else could test this for me.

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccXXX'

...

github-actions

Welcome @danu165 👋

It looks like this is your first Pull Request submission to the Terraform AWS Provider! If you haven’t already done so please make sure you have checked out our CONTRIBUTING guide and FAQ to make sure your contribution is adhering to best practice and has all the necessary elements in place for a successful approval.

Also take a look at our FAQ which details how we prioritize Pull Requests for inclusion.

Thanks again, and welcome to the community! 😃

YakDriver · 2021-04-15T00:59:53Z

@danu165 This looks promising. Let us know when you're ready for us to look it over.

danu165 · 2021-04-15T14:16:12Z

@YakDriver I think it's ready for review now. I put the WIP tag as I couldn't run a testacc due to weird behavior from my make fmt command. I've removed the tag if someone else can run that test for me.

DrFaust92

initial review

DrFaust92 · 2021-04-23T17:25:39Z

aws/data_source_aws_glue_data_catalog_encryption_settings.go

+	return &schema.Resource{
+		ReadContext: dataSourceAwsGlueDataCatalogEncryptionSettingsRead,
+		Schema: map[string]*schema.Schema{
+			"id": {


id attribute is implicit dont think we need this.

Is it also implicitly required? The goal here is to make id a required argument, so just want to make sure that behavior would remain the same if I take this out.

You are right, my mistake i see this is the convention in data sources to keep "id" explicit. ill try to run tests soon. code wise looks good

aws/data_source_aws_glue_connection.go

aws/data_source_aws_glue_data_catalog_encryption_settings.go

DrFaust92 · 2021-04-29T17:41:59Z

Getting this when trying to run tests:


    data_source_aws_glue_connection_test.go:18: Step 1/1 error: Error running apply: exit status 1
        
        Error: error creating Glue Connection (tf-testacc-glue-connection-mm9khn0lwzte1): InvalidInputException: PhysicalConnectionRequirements cannot be null

DrFaust92 · 2021-04-29T17:44:20Z

aws/data_source_aws_glue_connection_test.go

+
+func testAccDataSourceAwsGlueConnectionConfig(rName string) string {
+	return fmt.Sprintf(`
+resource "aws_glue_connection" "test" {


due to faling tests, lets change this to something that works on the resource accaptenece tests:

this is the basic config for connection resource tests

resource "aws_glue_connection" "test" { connection_properties = { JDBC_CONNECTION_URL = "jdbc:mysql://terraformacctesting.com/testdatabase" PASSWORD = "testpassword" USERNAME = "testusername" } name = "%s" }

Ok, thank you. Just pushed a commit with this test.

DrFaust92 · 2021-04-29T18:35:04Z

aws/data_source_aws_glue_connection_test.go

+					resource.TestCheckResourceAttrPair(datasourceName, "creation_time", resourceName, "creation_time"),
+					resource.TestCheckResourceAttrPair(datasourceName, "connection_type", resourceName, "connection_type"),
+					resource.TestCheckResourceAttrPair(datasourceName, "name", resourceName, "name"),
+				),


lets also add connection_properties check here as we added it.

I don't have connection_properties as part of the schema in the data source. It doesn't appear for every type of connection so I thought it might be best to leave it out.

DrFaust92 · 2021-04-29T18:45:44Z

aws/data_source_aws_glue_data_catalog_encryption_settings_test.go

+			{
+				Config: testAccDataSourceAwsGlueDataCatalogEncryptionSettingsConfig(),
+				Check: resource.ComposeTestCheckFunc(
+					testAccDataSourceAwsGlueDataCatalogEncryptionSettingsCheck(datasourceName),


from what i see in other datasource tests we don't need this check here. (the data source is not creating anything so there is nothing to check here)

I based this from data_source_aws_kms_key_test.go. Could you point me to another file whose structure I should copy?

DrFaust92 · 2021-04-29T18:46:31Z

aws/data_source_aws_glue_connection_test.go

+			{
+				Config: testAccDataSourceAwsGlueConnectionConfig(rName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccDataSourceAwsGlueConnectionCheck(datasourceName),


same as above resource

DrFaust92 · 2021-04-29T18:47:55Z

Hi @danu165.

now im getting:

Error: error decoding Glue Connection : expected ID in format CATALOG-ID:NAME, provided: 

  on terraform_plugin_test.tf line 12, in data "aws_glue_connection" "test":
  12: data "aws_glue_connection" "test" {


    data_source_aws_glue_connection_test.go:18: Step 1/1 error: Error running apply: exit status 1
        
        Error: error decoding Glue Connection : expected ID in format CATALOG-ID:NAME, provided: 
        
          on terraform_plugin_test.tf line 12, in data "aws_glue_connection" "test":
          12: data "aws_glue_connection" "test" {

Are you able to verify tests on your end? ill keep digging meanwhile

danu165 · 2021-04-30T00:02:01Z

@DrFaust92 I haven't been able to get tests working locally. As a note, when I run make fmt it edits almost every file in the repo. It seems to change the line separators for some reason. I then run make testacc TESTARGS='-run=TestAccDataSourceAwsGlueConnection_basic'. That command returns this error:

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccDataSourceAwsGlueConnection_basic -timeout 180m
=== RUN   TestAccDataSourceAwsGlueConnection_basic
=== PAUSE TestAccDataSourceAwsGlueConnection_basic
=== CONT  TestAccDataSourceAwsGlueConnection_basic
    data_source_aws_glue_connection_test.go:18: failed to create new working directory: symlink C:\Users\danu165\development\terraform-providers\terraform-provider-aws\aws\internal C:\Users\danu165\AppData\Local\Temp\plugintest52906
1170\work132527849\internal: A required privilege is not held by the client.
--- FAIL: TestAccDataSourceAwsGlueConnection_basic (3.33s)
FAIL
FAIL    github.com/terraform-providers/terraform-provider-aws/aws       19.216s
FAIL
make: *** [GNUmakefile:28: testacc] Error 1

So it seems my profile doesn't have permissions to create symlinks.

bflad · 2021-04-30T18:45:27Z

I've created an issue upstream in the project that handles the provider acceptance testing framework: hashicorp/terraform-plugin-sdk#759

Until that is resolved, unfortunately create symbolic link permissions on Windows may be required (or using another OS, such as WSL).

DrFaust92 · 2021-05-13T18:20:09Z

opened #19367 to address fixes with passing tests

DrFaust92 · 2021-05-17T20:00:40Z

Closing in favor of #19367. Thanks so much for your work @danu165!

github-actions · 2021-06-17T02:11:09Z

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

danu165 added 2 commits April 9, 2021 11:53

Add data source for aws_glue_connection

f72d07b

Add data source for aws_glue_data_catalog_encryption_settings

5f541f5

danu165 requested a review from a team as a code owner April 12, 2021 18:02

github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Apr 12, 2021

Add changelog

651972e

github-actions bot reviewed Apr 12, 2021

View reviewed changes

danu165 added 2 commits April 12, 2021 11:15

Fix linting

8ad215c

Add provider documentation

793d3a5

ghost added size/XL Managed by automation to categorize the size of a PR. documentation Introduces or discusses updates to documentation. and removed size/L Managed by automation to categorize the size of a PR. labels Apr 12, 2021

danu165 added 2 commits April 12, 2021 11:52

Fix lint of provider documentation

4cf7eac

Fix changelog

ed33d63

YakDriver assigned YakDriver and unassigned YakDriver Apr 14, 2021

YakDriver removed the needs-triage Waiting for first response or review from a maintainer. label Apr 15, 2021

YakDriver added the waiting-response Maintainers are waiting on response from community or contributor. label Apr 15, 2021

danu165 changed the title ~~[WIP] New Glue Data sources - Glue Connection and Glue Data Catalog Encryption Settings~~ New Glue Data sources - Glue Connection and Glue Data Catalog Encryption Settings Apr 15, 2021

ghost removed waiting-response Maintainers are waiting on response from community or contributor. labels Apr 15, 2021

DrFaust92 suggested changes Apr 23, 2021

View reviewed changes

ewbankkit reviewed Apr 26, 2021

View reviewed changes

aws/data_source_aws_glue_connection.go Show resolved Hide resolved

ewbankkit reviewed Apr 26, 2021

View reviewed changes

aws/data_source_aws_glue_connection.go Show resolved Hide resolved

ewbankkit reviewed Apr 26, 2021

View reviewed changes

aws/data_source_aws_glue_data_catalog_encryption_settings.go Show resolved Hide resolved

DrFaust92 self-assigned this Apr 26, 2021

DrFaust92 reviewed Apr 29, 2021

View reviewed changes

Fix test for glue connection

1ebcc82

DrFaust92 reviewed Apr 29, 2021

View reviewed changes

bflad mentioned this pull request Apr 30, 2021

Windows "failed to create new working directory" Without Create Symbolic Link Permissions hashicorp/terraform-plugin-sdk#759

Open

DrFaust92 mentioned this pull request May 13, 2021

d/glue_connection + glue_data_catalog_encryption_settings - new data sources #19367

Merged

DrFaust92 closed this May 17, 2021

github-actions bot locked as resolved and limited conversation to collaborators Jun 17, 2021

DrFaust92 removed their assignment Jun 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Glue Data sources - Glue Connection and Glue Data Catalog Encryption Settings #18802

New Glue Data sources - Glue Connection and Glue Data Catalog Encryption Settings #18802

danu165 commented Apr 12, 2021

github-actions bot left a comment

YakDriver commented Apr 15, 2021

danu165 commented Apr 15, 2021

DrFaust92 left a comment

DrFaust92 Apr 23, 2021

danu165 Apr 26, 2021

DrFaust92 Apr 26, 2021

DrFaust92 commented Apr 29, 2021 •

edited

Loading

DrFaust92 Apr 29, 2021

danu165 Apr 29, 2021

DrFaust92 Apr 29, 2021

danu165 Apr 29, 2021

DrFaust92 Apr 29, 2021

danu165 Apr 29, 2021

DrFaust92 Apr 29, 2021

DrFaust92 commented Apr 29, 2021 •

edited

Loading

danu165 commented Apr 30, 2021

bflad commented Apr 30, 2021

DrFaust92 commented May 13, 2021

DrFaust92 commented May 17, 2021

github-actions bot commented Jun 17, 2021

New Glue Data sources - Glue Connection and Glue Data Catalog Encryption Settings #18802

New Glue Data sources - Glue Connection and Glue Data Catalog Encryption Settings #18802

Conversation

danu165 commented Apr 12, 2021

Community Note

github-actions bot left a comment

Choose a reason for hiding this comment

YakDriver commented Apr 15, 2021

danu165 commented Apr 15, 2021

DrFaust92 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DrFaust92 commented Apr 29, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DrFaust92 commented Apr 29, 2021 • edited Loading

danu165 commented Apr 30, 2021

bflad commented Apr 30, 2021

DrFaust92 commented May 13, 2021

DrFaust92 commented May 17, 2021

github-actions bot commented Jun 17, 2021

DrFaust92 commented Apr 29, 2021 •

edited

Loading

DrFaust92 commented Apr 29, 2021 •

edited

Loading