Support storage HMAC key #261

julienduchesne · 2023-03-28T01:18:19Z

Issue: #51
This resource is needed to do S3-style access on GCS buckets

As this comment shows: #197 (comment), this resource causes an import cycle
However, we can resolve that by putting the resource in the cloudplatform package.
While this key is specific to storage, it is still just a product of a service account and it isn't linked to any storage resources. It is meant to be used by end-users and not other resources

sergenyalcin · 2023-04-04T11:33:09Z

/test-examples="examples/cloudplatform/storagehmackey.yaml"

julienduchesne · 2023-04-10T03:39:15Z

/test-examples="examples/cloudplatform/storagehmackey.yaml"

Sorry, I couldn't see the failure because I was on vacation and GH Actions deleted the logs. Can you run it again?

jeanduplessis · 2023-04-10T13:44:50Z

/test-examples="examples/cloudplatform/storagehmackey.yaml"

julienduchesne · 2023-04-10T23:53:14Z

Alright, I'll deploy a custom build on my cluster and test it there

julienduchesne · 2023-04-11T14:45:05Z

I just tested the feature in my cluster. Worked well! The issue with the example was that the SA name was too long

jeanduplessis · 2023-04-11T14:52:23Z

/test-examples="examples/cloudplatform/storagehmackey.yaml"

julienduchesne · 2023-04-11T15:02:08Z

It passed now. Anything for me to do about the breaking changes check? The check-examples one seems unrelated (failing on bigtable resources)

turkenf · 2023-06-08T19:52:03Z

Hi @julienduchesne, thank you for this PR.

This PR has conflicts, I tried to rebase but I don't have access to grafana/crossplane-provider-gcp. Could you please rebase the PR?

Issue: crossplane-contrib#51 This resource is needed to do S3-style access on GCS buckets As this comment shows: crossplane-contrib#197 (comment), this resource causes an import cycle However, we can resolve that by putting the resource in the `cloudplatform` package. While, this key is specific to storage, it is still just a product of a service account and isn't linked to any storage resources. Signed-off-by: Julien Duchesne <[email protected]>

Signed-off-by: Julien Duchesne <[email protected]>

julienduchesne · 2023-06-12T14:19:38Z

Hi @julienduchesne, thank you for this PR.

This PR has conflicts, I tried to rebase but I don't have access to grafana/crossplane-provider-gcp. Could you please rebase the PR?

Done!

turkenf · 2023-06-12T17:50:48Z

/test-examples="examples/cloudplatform/storagehmackey.yaml"

turkenf · 2023-06-22T20:56:10Z

config/cloudplatform/config.go

+		r.References["service_account_email"] = config.Reference{
+			Type:      "ServiceAccount",
+			Extractor: common.ExtractEmailFuncPath,


Suggested change

r.References["service_account_email"] = config.Reference{

Type: "ServiceAccount",

Extractor: common.ExtractEmailFuncPath,

r.References["service_account_email"] = config.Reference{

TerraformName: "google_service_account",

Extractor: `github.com/upbound/upjet/pkg/resource.ExtractParamPath("email",true)`,

We have a function that we use for these cases. I think there is no need for an extra function here, the above configuration will suffice.

turkenf · 2023-06-22T20:58:12Z

config/common/config.go

+	// ExtractEmailFuncPath holds the GCP resource email extractor func name
+	ExtractEmailFuncPath = "github.com/upbound/provider-gcp/config/common.ExtractEmail()"


We can remove these lines.

turkenf · 2023-06-22T20:58:19Z

config/common/config.go

+
+// ExtractEmail extracts the value of `status.atProvider.email`
+// from a Terraformed resource. If mr is not a Terraformed
+// resource, returns an empty string.
+func ExtractEmail() reference.ExtractValueFn {
+	return func(mg resource.Managed) string {
+		paved, err := fieldpath.PaveObject(mg)
+		if err != nil {
+			return ""
+		}
+		r, err := paved.GetString("status.atProvider.email")
+		if err != nil {
+			return ""
+		}
+		return r
+	}
+}


We can remove these lines.

turkenf · 2023-06-22T21:03:24Z

config/overrides.go

+	// Avoid an import cycle, this is a key used for S3-style auth. It's meant to be used by end users, not other resources.
+	"google_storage_hmac_key$": ReplaceGroupWords("cloudplatform", 0),


I verified that if we don't make a group change it causes an import cycle:

But I'm not sure if this is the right solution here. @ulucinar, @sergenyalcin what do you think about this?

@turkenf as far as I understand, the reason of this import cycle is reference addition in the config file.

In the context of API groups, I don't think this is the right solution approach, since we depend on external providers' groups.

Until today, we have chosen the method of removing references in these cases. I am aware that this is not an ideal solution either, but in order to solve this problem in the long term, it is necessary to address it at the upjet level.

@turkenf as far as I understand, the reason of this import cycle is reference addition in the config file.

@sergenyalcin in fact, even if no reference is defined in the config file, we encounter an import cycle error because the same resource is automatically injected with references.

We can reset it with the following configuration in config/storage/config.go and in this case, we do not encounter an import cycle error:

p.AddResourceConfigurator("google_storage_hmac_key", func(r *config.Resource) { r.References["service_account_email"] = config.Reference{} })

Should we proceed this way?

Emptying the reference does work. However, I have one issue. The service account email is made up of the SA name + the GCP project name. I do not have the latter, so I cannot create an example

Any way to fix that?

turkenf · 2023-07-18T08:57:33Z

/test-examples="examples/cloudplatform/storagehmackey.yaml"

Anthony-Bible · 2024-03-07T15:11:16Z

I'm curious about the reason for closing this PR. Was it due to breaking changes or requiring a fix from an upstream dependency (upjet)? This feature seems very important for s3 api compatibility, which is essential for organizations that use both providers for their buckets.

jake-ciolek · 2024-06-18T13:15:16Z

For those interested, I've submitted a new PR targeting this resource, see #546

julienduchesne force-pushed the julienduchesne/support-storage-hmac branch from a54e42a to 23bed65 Compare March 28, 2023 01:34

julienduchesne marked this pull request as ready for review March 28, 2023 01:51

julienduchesne requested review from ulucinar and sergenyalcin as code owners March 28, 2023 01:51

julienduchesne force-pushed the julienduchesne/support-storage-hmac branch from 23bed65 to 98a58e1 Compare April 10, 2023 03:38

julienduchesne force-pushed the julienduchesne/support-storage-hmac branch from 990f46a to f4ec683 Compare April 11, 2023 14:44

julienduchesne force-pushed the julienduchesne/support-storage-hmac branch from f4ec683 to 2d1289c Compare May 2, 2023 13:56

julienduchesne added 2 commits June 12, 2023 10:18

Use shorter example name. It was failing due to a SA name length

a5bb2aa

Signed-off-by: Julien Duchesne <[email protected]>

julienduchesne force-pushed the julienduchesne/support-storage-hmac branch from 2d1289c to a5bb2aa Compare June 12, 2023 14:18

turkenf reviewed Jun 22, 2023

View reviewed changes

julienduchesne closed this Nov 30, 2023

jake-ciolek mentioned this pull request Jun 18, 2024

Add google_storage_hmac_key #546

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support storage HMAC key #261

Support storage HMAC key #261

julienduchesne commented Mar 28, 2023 •

edited

Loading

sergenyalcin commented Apr 4, 2023

julienduchesne commented Apr 10, 2023

jeanduplessis commented Apr 10, 2023

julienduchesne commented Apr 10, 2023

julienduchesne commented Apr 11, 2023

jeanduplessis commented Apr 11, 2023

julienduchesne commented Apr 11, 2023

turkenf commented Jun 8, 2023

julienduchesne commented Jun 12, 2023

turkenf commented Jun 12, 2023

turkenf Jun 22, 2023

turkenf Jun 22, 2023

turkenf Jun 22, 2023

turkenf Jun 22, 2023

sergenyalcin Jun 23, 2023

turkenf Jun 23, 2023

julienduchesne Jun 26, 2023

turkenf commented Jul 18, 2023

Anthony-Bible commented Mar 7, 2024

jake-ciolek commented Jun 18, 2024

		// ExtractEmailFuncPath holds the GCP resource email extractor func name
		ExtractEmailFuncPath = "github.com/upbound/provider-gcp/config/common.ExtractEmail()"

		// Avoid an import cycle, this is a key used for S3-style auth. It's meant to be used by end users, not other resources.
		"google_storage_hmac_key$": ReplaceGroupWords("cloudplatform", 0),

Support storage HMAC key #261

Support storage HMAC key #261

Conversation

julienduchesne commented Mar 28, 2023 • edited Loading

sergenyalcin commented Apr 4, 2023

julienduchesne commented Apr 10, 2023

jeanduplessis commented Apr 10, 2023

julienduchesne commented Apr 10, 2023

julienduchesne commented Apr 11, 2023

jeanduplessis commented Apr 11, 2023

julienduchesne commented Apr 11, 2023

turkenf commented Jun 8, 2023

julienduchesne commented Jun 12, 2023

turkenf commented Jun 12, 2023

turkenf Jun 22, 2023

Choose a reason for hiding this comment

turkenf Jun 22, 2023

Choose a reason for hiding this comment

turkenf Jun 22, 2023

Choose a reason for hiding this comment

turkenf Jun 22, 2023

Choose a reason for hiding this comment

sergenyalcin Jun 23, 2023

Choose a reason for hiding this comment

turkenf Jun 23, 2023

Choose a reason for hiding this comment

julienduchesne Jun 26, 2023

Choose a reason for hiding this comment

turkenf commented Jul 18, 2023

Anthony-Bible commented Mar 7, 2024

jake-ciolek commented Jun 18, 2024

julienduchesne commented Mar 28, 2023 •

edited

Loading