Add `kubernetes_resource` data source #1548

jrhouston · 2021-12-13T21:12:13Z

Description

This PR adds a generic kubernetes_resource data source as an analogue to the kubernetes_manifest resource.

Acceptance tests

Have you added an acceptance test for the functionality being added?
Have you run the acceptance tests on this branch?

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccXXX'

...

Release Note

Release note for CHANGELOG:

Add kubernetes_resource data source

References

Community Note

Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
If you are interested in working on this issue or have submitted a pull request, please leave a comment

jrhouston · 2021-12-14T16:57:56Z

website/docs/d/resource.html.markdown

+### Example: Get data from a ConfigMap
+
+```hcl
+data "kubernetes_resource" "example" {


@alexsomesan open to thoughts on the naming here, I didn't use manifest because that specifically refers to the YAML file you apply to the cluster whereas here we're retrieving an API resource so I thought resource made sense. kubernetes_object would also work too.

I'm with you. This is a good choice of name. Using manifest would likely be more confusing. Also, this datasource could be used together with the native resources (non kubernetes_manifest) and then it makes even more sense to call it like this.

jrhouston · 2021-12-14T17:01:06Z

website/docs/d/resource.html.markdown

+
+```hcl
+data "kubernetes_resource" "example" {
+  apiVersion = "v1"


Worth noting that we're going to break the snake_case convention that is used everywhere else (and all other providers too) for resource attributes. api_version would be fine here, just that the camelCase convention is probably more familiar to users of this provider.

I honestly am ok with either option. There are valid points, which you raised, for both of them.
I'm with you there that keeping with the spirit of this resources trying to track as close as possible to native K8s, this looks like the slightly better option.

I've changed this to api_version after my conversation with DevEx – they recommended not breaking the convention.

alexsomesan

I like it! I played around with the datasource and it works nicely as expected.

I dropped some observations about UX down below, but structurally this is great work!
Thanks! a lot!

alexsomesan · 2022-01-26T19:50:30Z

manifest/provider/datasource.go

+
+	gvr, err := getGVR(apiVersion, kind, rm)
+	if err != nil {
+		return resp, err


It would be nice to keep consistent and return nice Diagnostics here that wrap this error. We should strive to have zero instances of raw error returns to the protocol.

alexsomesan · 2022-01-26T19:51:48Z

manifest/provider/datasource.go

+	gvk := gvr.GroupVersion().WithKind(kind)
+	ns, err := IsResourceNamespaced(gvk, rm)
+	if err != nil {
+		return resp, err


same observation about error returned here.

alexsomesan · 2022-01-26T20:08:05Z

manifest/provider/datasource.go

+		return resp, nil
+	}
+
+	objectType, err := s.TFTypeFromOpenAPI(ctx, gvk, false)


A point could be made that this should best happen before the actual API Get call. Reason being that it might serve as validation for the correctness of the input attributes independently of potential permission errors on the resource Get call.
What I'm not sure about is whether client-go distinguishes (with different error codes) between a RBAC permission issue on a valid path and an invalid path (likely also reported by 'not found').

alexsomesan · 2022-01-26T20:08:35Z

manifest/provider/datasource.go

+
+	objectType, err := s.TFTypeFromOpenAPI(ctx, gvk, false)
+	if err != nil {
+		return resp, fmt.Errorf("failed to determine data source type: %s", err)


Diagnostic vs raw error here too

...and a few more spot down below.

alexsomesan · 2022-01-26T20:10:30Z

website/docs/d/resource.html.markdown

+
+```hcl
+data "kubernetes_resource" "example" {
+  apiVersion = "v1"


I honestly am ok with either option. There are valid points, which you raised, for both of them.
I'm with you there that keeping with the spirit of this resources trying to track as close as possible to native K8s, this looks like the slightly better option.

alexsomesan · 2022-01-26T20:12:28Z

website/docs/d/resource.html.markdown

+### Example: Get data from a ConfigMap
+
+```hcl
+data "kubernetes_resource" "example" {


I'm with you. This is a good choice of name. Using manifest would likely be more confusing. Also, this datasource could be used together with the native resources (non kubernetes_manifest) and then it makes even more sense to call it like this.

jrhouston · 2022-02-01T03:04:49Z

As discussed, I have changed metadata to a block instead of an object – this should be good to go.

alexsomesan · 2022-02-03T11:34:35Z

Looks good! How do you feel about the Diagnostics suggestions I made earlier?

jrhouston · 2022-02-04T16:47:19Z

Yeah sounds good, I've wrapped all the errors in diagnostics.

alexsomesan

LGTM

jrhouston · 2022-02-17T21:06:27Z

Looks like the tests are failing because I missed something when rebasing – will fix.

github-actions · 2022-03-26T02:16:27Z

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.

github-actions bot added documentation size/XL labels Dec 13, 2021

jrhouston force-pushed the manifest-datasource branch from 7e89a96 to 71608a9 Compare December 14, 2021 10:08

jrhouston changed the title ~~Manifest datasource~~ Add kubernetes_resource data source Dec 14, 2021

jrhouston commented Dec 14, 2021

View reviewed changes

alexsomesan reviewed Jan 26, 2022

View reviewed changes

jrhouston force-pushed the manifest-datasource branch 2 times, most recently from 95cba7e to 7d25484 Compare February 1, 2022 03:17

jrhouston force-pushed the manifest-datasource branch from 7d25484 to 4115bde Compare February 11, 2022 23:00

alexsomesan approved these changes Feb 17, 2022

View reviewed changes

alexsomesan mentioned this pull request Feb 17, 2022

Status info missing from kubernetes_manifest object attribute after apply #1605

Closed

jrhouston added 2 commits February 22, 2022 12:00

Add kubernetes_resource data source

0551669

Fix acctest

06333fc

jrhouston force-pushed the manifest-datasource branch from 245dc82 to 06333fc Compare February 23, 2022 06:02

Fix docs

cfbd1cf

jrhouston merged commit bc4290b into main Feb 23, 2022

jrhouston deleted the manifest-datasource branch February 23, 2022 06:51

github-actions bot locked as resolved and limited conversation to collaborators Mar 26, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `kubernetes_resource` data source #1548

Add `kubernetes_resource` data source #1548

jrhouston commented Dec 13, 2021

jrhouston Dec 14, 2021

alexsomesan Jan 26, 2022

jrhouston Dec 14, 2021

alexsomesan Jan 26, 2022

jrhouston Feb 1, 2022

alexsomesan left a comment

alexsomesan Jan 26, 2022

alexsomesan Jan 26, 2022

alexsomesan Jan 26, 2022

alexsomesan Jan 26, 2022

alexsomesan Jan 26, 2022

alexsomesan Jan 26, 2022

jrhouston commented Feb 1, 2022 •

edited

Loading

alexsomesan commented Feb 3, 2022

jrhouston commented Feb 4, 2022

alexsomesan left a comment

jrhouston commented Feb 17, 2022

github-actions bot commented Mar 26, 2022

Add kubernetes_resource data source #1548

Add kubernetes_resource data source #1548

Conversation

jrhouston commented Dec 13, 2021

Description

Acceptance tests

Release Note

References

Community Note

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

alexsomesan 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrhouston commented Feb 1, 2022 • edited Loading

alexsomesan commented Feb 3, 2022

jrhouston commented Feb 4, 2022

alexsomesan left a comment

Choose a reason for hiding this comment

jrhouston commented Feb 17, 2022

github-actions bot commented Mar 26, 2022

Add `kubernetes_resource` data source #1548

Add `kubernetes_resource` data source #1548

jrhouston commented Feb 1, 2022 •

edited

Loading