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.

Sign up for GitHub

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

Jump to bottom

feat(datacatalog): add taxonomy & policy tags resources #643

Merged
merged 3 commits into from
Oct 31, 2024
Merged

feat(datacatalog): add taxonomy & policy tags resources #643

merged 3 commits into from
Oct 31, 2024

Conversation

nilan3
Copy link
Contributor

@nilan3 nilan3 commented Oct 31, 2024

Description of your changes

Add resources for the following terraform resources:

google_data_catalog_taxonomy
google_data_catalog_policy_tag

Fixes #

I have:

  • Read and followed Crossplane's [contribution process].
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

Running Uptest locally against a personal sandbox GCP project

UPTEST_EXAMPLE_LIST="examples/datacatalog/v1beta1/taxonomy.yaml"

--- PASS: kuttl (50.28s)
--- PASS: kuttl/harness (0.00s)
--- PASS: kuttl/harness/case (49.02s)
PASS
10:45:53 [ OK ] running automated tests

image
image

@nilan3 nilan3 marked this pull request as ready for review October 31, 2024 13:16
@nilan3
Copy link
Contributor Author

nilan3 commented Oct 31, 2024

/test-examples="examples/datacatalog/v1beta1/taxonomy.yaml"

1 similar comment
@jeanduplessis
Copy link
Collaborator

/test-examples="examples/datacatalog/v1beta1/taxonomy.yaml"

@nilan3 nilan3 force-pushed the feature/datacatalog_taxonomy_policytag branch 2 times, most recently from 20108ea to 392d073 Compare October 31, 2024 14:16
@nilan3
Copy link
Contributor Author

nilan3 commented Oct 31, 2024

Could we run the test-example again pls?

@jeanduplessis
Copy link
Collaborator

/test-examples="examples/datacatalog/v1beta1/taxonomy.yaml"

@nilan3
Copy link
Contributor Author

nilan3 commented Oct 31, 2024

looks like the e2e test failed due to permissions

'create failed: async create failed: failed to create the resource:
    logger.go:42: 14:59:52 | case/0-apply |         [***0 Error creating Taxonomy: googleapi: Error 403: The caller does not have
    logger.go:42: 14:59:52 | case/0-apply |         permission  []***]'

@turkenf
Copy link
Collaborator

turkenf commented Oct 31, 2024

Closing and re-opening pipeline work.

@turkenf turkenf closed this Oct 31, 2024
@turkenf turkenf reopened this Oct 31, 2024
nilan3 added 3 commits October 31, 2024 20:47
@nilan3 @turkenf
update gitignore
e8399d0 
Signed-off-by: nilan3 <nilanthanb1994@gmail.com>
@nilan3 @turkenf
enable taxonomy & policy tag
b45de02 
Signed-off-by: nilan3 <nilanthanb1994@gmail.com>
@nilan3 @turkenf
make reviewable
7ecb3f2 
Signed-off-by: nilan3 <nilanthanb1994@gmail.com>
@turkenf turkenf force-pushed the feature/datacatalog_taxonomy_policytag branch from 392d073 to 7ecb3f2 Compare October 31, 2024 17:51
@turkenf
Copy link
Collaborator

turkenf commented Oct 31, 2024 •
edited
Loading

/test-examples="examples/datacatalog/v1beta1/taxonomy.yaml"

https://github.com/crossplane-contrib/provider-upjet-gcp/actions/runs/11619091847

@turkenf
Copy link
Collaborator

turkenf commented Oct 31, 2024 •
edited
Loading

/test-examples="examples/datacatalog/v1beta1/policytag.yaml"

https://github.com/crossplane-contrib/provider-upjet-gcp/actions/runs/11619412833

turkenf
turkenf approved these changes Oct 31, 2024
View reviewed changes
Copy link
Collaborator

@turkenf turkenf left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution @nilan3, LGTM 🙌

I rebased your PR to main to get bug fixes and resolve conflicts. After assigning the required role to our test account, the permission issue was resolved and the tests passed 🎉

config/externalname.go
Comment on lines +379 to +382
// {{name}} projects/{project}/locations/{region}/taxonomies/{taxonomy}
"google_data_catalog_taxonomy": config.IdentifierFromProvider,
// {{name}} projects/{project}/locations/{region}/taxonomies/{taxonomy}/policyTags/{policytag}
"google_data_catalog_policy_tag": config.IdentifierFromProvider,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Note: Since taxonomy and policytag are system generated identifiers, we cannot use TemplatedStringAsIdentifier here: id: projects/official-provider-testing/locations/eu/taxonomies/2064837193133375339/policyTags/1548338660528116821

@turkenf turkenf merged commit f3202a4 into crossplane-contrib:main Oct 31, 2024
11 checks passed
@nilan3
Copy link
Contributor Author

nilan3 commented Nov 1, 2024

Thank you for your contribution @nilan3, LGTM 🙌

I rebased your PR to main to get bug fixes and resolve conflicts. After assigning the required role to our test account, the permission issue was resolved and the tests passed 🎉

Thanks @turkenf

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@turkenf turkenf turkenf approved these changes

@ulucinar ulucinar Awaiting requested review from ulucinar ulucinar is a code owner

@sergenyalcin sergenyalcin Awaiting requested review from sergenyalcin sergenyalcin is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nilan3 @jeanduplessis @turkenf