-
Notifications
You must be signed in to change notification settings - Fork 30
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
feat: Account for custom S3 compatible storages (add optional parameters to AWS adapter) #11
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,102 @@ | ||
AWSTemplateFormatVersion: "2010-09-09" | ||
Parameters: | ||
GitLfsUsername: | ||
Type: String | ||
Description: Username for authenticating against Git LFS endpoint | ||
GitLfsPassword: | ||
Type: String | ||
Description: Password for authenticating against Git LFS endpoint | ||
Outputs: | ||
LfsEndpoint: | ||
Description: The Git LFS endpoint to use | ||
Value: !Sub 'https://${RestApi}.execute-api.${AWS::Region}.amazonaws.com/lfs' | ||
Resources: | ||
StorageBucket: | ||
Type: AWS::S3::Bucket | ||
DeletionPolicy: Retain | ||
RestApi: | ||
Type: AWS::ApiGateway::RestApi | ||
Properties: | ||
Body: | ||
swagger: '2.0' | ||
info: | ||
description: 'Describes a proxy to a Lambda function to sign S3 requests.' | ||
title: 'Git LFS REST API' | ||
version: '1.0.0' | ||
paths: | ||
/{proxy+}: | ||
x-amazon-apigateway-any-method: | ||
produces: | ||
- application/json | ||
parameters: | ||
- name: proxy | ||
in: path | ||
required: true | ||
type: string | ||
responses: {} | ||
x-amazon-apigateway-integration: | ||
responses: | ||
default: | ||
statusCode: 200 | ||
uri: !Sub 'arn:aws:apigateway:${AWS::Region}:lambda:path/2015-03-31/functions/arn:aws:lambda:${AWS::Region}:${AWS::AccountId}:function:${SigningLambda}/invocations' | ||
passthroughBehavior: when_no_match | ||
httpMethod: POST | ||
contentHandling: CONVERT_TO_TEXT | ||
type: aws_proxy | ||
Description: Git LFS endpoint | ||
FailOnWarnings: true | ||
Name: !Ref AWS::StackName | ||
RestDeployment: | ||
Type: AWS::ApiGateway::Deployment | ||
Properties: | ||
RestApiId: !Ref RestApi | ||
StageName: lfs | ||
SigningLambda: | ||
Type: AWS::Lambda::Function | ||
Properties: | ||
Code: | ||
S3Bucket: !Sub 'ae-infrastructure-${AWS::Region}' | ||
S3Key: git-lfs/3.0.0/Estranged.Lfs.Hosting.Lambda.zip | ||
Description: Generates S3 signed URLs for Git LFS | ||
FunctionName: !Ref AWS::StackName | ||
Handler: Estranged.Lfs.Hosting.Lambda::Estranged.Lfs.Hosting.Lambda.LambdaEntryPoint::FunctionHandlerAsync | ||
MemorySize: 512 | ||
Role: !GetAtt SigningLambdaRole.Arn | ||
Runtime: dotnetcore3.1 | ||
Timeout: 30 | ||
Environment: | ||
Variables: | ||
LFS_BUCKET: !Ref StorageBucket | ||
LFS_USERNAME: !Ref GitLfsUsername | ||
LFS_PASSWORD: !Ref GitLfsPassword | ||
GITHUB_ORGANISATION: !Ref GitOrganisation | ||
GITHUB_REPOSITORY: !Ref GitHubRepositoryVariable | ||
BITBUCKET_WORKSPACE: !Ref BitBucketWorkspaceVariable | ||
BITBUCKET_REPOSITORY: !Ref BitBucketRepositoryVariable | ||
S3Region: !Ref S3_REGION | ||
S3ServiceURL: !Ref S3_SERVICE_URL | ||
S3AccessKey: !Ref S3_ACCESS_KEY | ||
S3AccessSecret: !Ref S3_ACCESS_SECRET | ||
SigningLambdaGatewayPermission: | ||
Type: AWS::Lambda::Permission | ||
Properties: | ||
Action: lambda:InvokeFunction | ||
FunctionName: !GetAtt SigningLambda.Arn | ||
Principal: apigateway.amazonaws.com | ||
SourceArn: !Sub arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${RestApi}/* | ||
SigningLambdaRole: | ||
Type: AWS::IAM::Role | ||
Properties: | ||
AssumeRolePolicyDocument: | ||
Version: "2012-10-17" | ||
Statement: | ||
- | ||
Effect: Allow | ||
Principal: | ||
Service: | ||
- lambda.amazonaws.com | ||
Action: | ||
- sts:AssumeRole | ||
ManagedPolicyArns: | ||
- arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole | ||
- arn:aws:iam::aws:policy/AmazonS3FullAccess |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -43,6 +43,8 @@ public async Task<SignedBlob> UriForDownload(string oid, CancellationToken token | |
} | ||
catch (AmazonS3Exception ex) | ||
{ | ||
Console.WriteLine($"[ERROR] - {ex.StatusCode} - {ex.Message}"); | ||
Console.WriteLine(ex.StackTrace); | ||
return new SignedBlob | ||
{ | ||
ErrorCode = (int)ex.StatusCode, | ||
|
@@ -62,12 +64,8 @@ public Task<SignedBlob> UriForUpload(string oid, long size, CancellationToken to | |
{ | ||
return Task.FromResult(new SignedBlob | ||
{ | ||
Uri = MakePreSignedUrl(oid, HttpVerb.PUT, BlobConstants.UploadMimeType), | ||
Expiry = config.Expiry, | ||
Headers = new Dictionary<string, string> | ||
{ | ||
{"Content-Type", BlobConstants.UploadMimeType} | ||
} | ||
Comment on lines
-67
to
-70
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The S3 supports headers, if the API in use does not support headers, it's not S3 compatible There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It does support headers, but it's Why are use using it in the first place? I found this example that's use content type This small PR would open to many cloud providers which are compatible with S3. I'm sure the community will understand you give support mostly for AWS, and they'll contribute if other cloud providers bring problems (like I did). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You said you couldn't test it, but maybe you could test it on AWS S3. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another use case that could be useful for people with this. It allows to use an AWS S3 bucket that's outside the stack, even in another region 😉 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hello @alanedwardes, have you been able to try testing as I suggested? |
||
Uri = MakePreSignedUrl(oid, HttpVerb.PUT, null), | ||
Expiry = config.Expiry | ||
}); | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The point of these being packages on NuGet is so that this kind of granular customisation can happen outside of this repository (e.g. in your own application). I think this is too application specific to be useful in the sample application.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are just using the default values, which only restrict to AWS S3.
It opens the configuration with optional parameters without breaking the existing AWS one.
With this solution, anyone could just clone the repo and is ready to go (required only to change the parameters to fit its cloud provider).