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.

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

Feature/s3 c 9226 create bucket #4

Closed
wants to merge 9 commits into from

Conversation

anurag4DSB
Copy link
Collaborator

@anurag4DSB anurag4DSB commented Sep 13, 2024

feature: Create Bucket using provisioner

  • Updated provisioner to implement create bucket functionality
  • Added an S3 client to interact with S3
  • Added logs to the methods not implemented yet
  • Added placeholder for accepting TLS certificates for S3 endpoint.
  • Added mock clients and tests for provisioner

- Added Dockerfile, Makefile and CI workflows
- Added tests for DriverInfo in identity server
Issue: S3C-9222
@anurag4DSB anurag4DSB marked this pull request as ready for review September 13, 2024 07:04
- Updated provisioner to implement create bucket functionality
- Added an S3 client to interact with S3
- Added logs to the methods not implemented yet
- Added placeholder for accepting TLS certificates for S3 endpoint.

Issue: S3C-9226
go.mod Outdated
)

require (
github.com/aws/aws-sdk-go v1.55.5
Copy link

Choose a reason for hiding this comment

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

The AWS recommendation is to move to V2 https://github.com/aws/aws-sdk-go-v2, end of support is in less than a year for V1.

@@ -0,0 +1,120 @@
/*
Copy link

Choose a reason for hiding this comment

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

Should this be in a LICENSE file?

@@ -0,0 +1,5317 @@
// Code generated by MockGen. DO NOT EDIT.
Copy link

Choose a reason for hiding this comment

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

We should at least document how this is generated, for repeatability. How about adding it in the build step of the Makefile?

return s3Client, nil
}

func fetchS3Parameters(secretData map[string][]byte) (string, string, string, string, string, error) {

Choose a reason for hiding this comment

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

May be worth creating a private type s3Params struct{ AccessKey string ...} returned by this function.

}

// configureTLSTransport sets up the HTTP transport with TLS support
func configureTLSTransport(certData []byte, skipTLS bool) *http.Transport {

Choose a reason for hiding this comment

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

Suggested change
func configureTLSTransport(certData []byte, skipTLS bool) *http.Transport {
func configureTLSTransport(certData []byte, skipVerify bool) *http.Transport {

Base automatically changed from feature/S3C-9222-setup-CI to main October 21, 2024 08:44
@anurag4DSB
Copy link
Collaborator Author

As too much rework is needed, I am gonna divide this into 2 PRs
Major rework needed:

  • Use AWS SDK v2
  • Use Ginkgo for tests

First PR: #6

@anurag4DSB anurag4DSB closed this Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants