feat: Add GCS support #400
Conversation
- Adding helper structs and tags
- These will be unmarshalled into via CLI from params.yaml. - Added helper function, MarshalToYaml.
- Returns client that can interact with google cloud storage.
S3 or GCS are set. - If not, throw an error.
- GCS has necessary credentials in serviceAccount as json
Adding explicit error checking for opts.SetRange.
Added switch statement for GCS versus S3.
- Do not write the value of ServiceAccount to Config Map. This exposes credentials. - Properly write the reference the argo expects. The reference points to the secret that contains the credentials.
…o use. Adding another field to struct, which will store the ServiceAccountJSON. - This will be generated by the API and used by the API for the GCS client.
| google.golang.org/genproto v0.0.0-20200430143042-b979b6f78d84 | ||
| google.golang.org/grpc v1.28.0 | ||
| google.golang.org/protobuf v1.22.0 | ||
| gopkg.in/yaml.v2 v2.2.8 | ||
| gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776 |
There was a problem hiding this comment.
Why do we need this too? Now we have are using 4 different libraries. Can you use "sigs.k8s.io/yaml" instead? I'd like to eventually refactor everything to use that.
There was a problem hiding this comment.
Going to create a separate ticket to investigate this.
- We use yaml.v3 to support reading (and writing) YAML with comments, inline comments, and other metadata.
- We use pkg/config_types.go as a library in CLI to marshal / unmarshal
params.yaml - Same package is used in core to dynamically generate the s3 access and secret keys, for Workflows
I think, ideally, we should use yaml.v3
the secret reference and secret key.
aleksandrmelnikov
changed the title
Added GetObject function to be consistent with S3 functionality. Note - pkg/client.go, line 88 passes in the JSON. If ArtifactRepositoryGCSCondig was imported inside gcs.NewClient code, an import cycle would be created.
pkg/config_types.go
Outdated
| AccessKey string | ||
| Secretkey string | ||
| AccessKey string `yaml:"accessKey"` | ||
| Secretkey string `yaml:"secretKey"` |
There was a problem hiding this comment.
Do we need this if we have ArtifactRepositoryS3Provider? Can't we just move AccessKey and SecretKey fields to ArtifactRepositoryS3Provider and use that across the board?
There was a problem hiding this comment.
should be
SecretKey
Sorry, do you mean the yaml: "secretKey"?
There was a problem hiding this comment.
@aleksandrmelnikov the K in the actual field name is lower case now, should be uppercase.
There was a problem hiding this comment.
@rushtehrani I was able to move the attributes from S3Config to S3Provider.
Did you to remove the S3Config struct when you said "use across the board"?
There was a problem hiding this comment.
Made changes.
- Adding comments - Refactoring variables names - Fixing return variable order
rushtehrani
changed the title
What this PR does:
Updated code to support Google cloud storage if available.
Which issue(s) this PR fixes:
Fixes #331
Special notes for your reviewer: