Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions .github/actions/setup-smithy/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ runs:
steps:
- name: Install Smithy CLI
shell: bash
env:
SMITHY_VERSION: '1.61.0'
run: |
# Extract Smithy version from smithy-build.json
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not directly related with the pr, but this is nicer as we will only have one source of truth for smithy version.
I could definitely see someone updating smithy version here and forgetting about updating it in smithy-build.json

SMITHY_VERSION=$(jq -r '.maven.dependencies[] | select(contains("smithy-aws-traits")) | split(":")[2]' smithy-build.json)
echo "Installing Smithy CLI version ${SMITHY_VERSION}"

mkdir -p smithy-install/smithy
curl -L https://github.com/smithy-lang/smithy/releases/download/${SMITHY_VERSION}/smithy-cli-linux-x86_64.zip -o smithy-install/smithy-cli-linux-x86_64.zip
unzip -qo smithy-install/smithy-cli-linux-x86_64.zip -d smithy-install
Expand Down
96 changes: 56 additions & 40 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
@@ -1,66 +1,82 @@
---
name: release
run-name: release ${{ inputs.tag }}
name: Release
run-name: Release

on:
# Uncomment to test your work as a release before it's merged
# push:
# branches:
# - improvement/CLDSRVCLT-X
workflow_dispatch:
inputs:
tag:
description: 'Tag to be released (e.g., 1.0.0)'
required: true

jobs:
build-and-tag:
name: Build and tag
build:
name: Build
runs-on: ubuntu-latest
permissions:
contents: write

outputs:
version: ${{ steps.package-version.outputs.version }}
steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Get version from package.json
id: package-version
run: echo "version=$(node -p "require('./package.json').version")" >> $GITHUB_OUTPUT

- name: Setup and Build
uses: ./.github/actions/setup-and-build

- name: Create Tag with Build Artifacts
run: |
# Configure git user to the GitHub Actions bot
git config --global user.name "github-actions[bot]"
git config --global user.email "github-actions[bot]@users.noreply.github.com"
- name: Upload build artifacts
uses: actions/upload-artifact@v4
with:
name: build-artifacts
path: |
dist/
build/

# Force add the build folders (even if they are in .gitignore)
git add -f dist build
publish-npm:
name: Publish to npm registry
runs-on: ubuntu-latest
needs: build
permissions:
contents: read
id-token: write
steps:
- name: Checkout code
uses: actions/checkout@v4

# Determine tag name
TAG_NAME="${{ inputs.tag }}"
if [ -z "$TAG_NAME" ]; then
TAG_NAME="test-${{ github.sha }}"
fi
- name: Download build artifacts
uses: actions/download-artifact@v4
with:
name: build-artifacts

# Commit the build artifacts
git commit -m "Build artifacts for version $TAG_NAME"
- name: Setup Node.js for npm registry
uses: actions/setup-node@v4
with:
node-version: '24'
registry-url: 'https://registry.npmjs.org'

# Create the tag
git tag $TAG_NAME
- name: Publish to npm with provenance
run: npm publish --provenance --tag latest

# Push the tag to the repository
git push origin $TAG_NAME
create-release:
name: Create GitHub Release
runs-on: ubuntu-latest
needs: [build, publish-npm]
permissions:
contents: write
steps:
- name: Checkout code
uses: actions/checkout@v4

# Export tag name for next step
echo "tag_name=$TAG_NAME" >> $GITHUB_OUTPUT
id: create_tag
- name: Create Git Tag
run: |
git config --global user.name "github-actions[bot]"
git config --global user.email "github-actions[bot]@users.noreply.github.com"
git tag ${{ needs.build.outputs.version }}
git push origin ${{ needs.build.outputs.version }}

- name: Create GitHub Release
uses: softprops/action-gh-release@v2
with:
tag_name: ${{ steps.create_tag.outputs.tag_name }}
name: Release ${{ steps.create_tag.outputs.tag_name }}
draft: false
prerelease: false
tag_name: ${{ needs.build.outputs.version }}
name: Release ${{ needs.build.outputs.version }}
generate_release_notes: true
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
14 changes: 14 additions & 0 deletions models/quotas/deleteBucketQuota.smithy
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
$version: "2.0"

namespace cloudserver.bucketquota

@http(method: "DELETE", uri: "/{Bucket}?quota=true")
@idempotent
operation DeleteBucketQuota {
input := {
@required
@httpLabel
Bucket: String
}
output := {}
}
17 changes: 17 additions & 0 deletions models/quotas/getBucketQuota.smithy
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
$version: "2.0"

namespace cloudserver.bucketquota

@http(method: "GET", uri: "/{Bucket}?quota=true")
@readonly
operation GetBucketQuota {
input := {
@required
@httpLabel
Bucket: String
}
output := {
Name: String
Quota: Integer
}
}
20 changes: 20 additions & 0 deletions models/quotas/updateBucketQuota.smithy
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
$version: "2.0"

namespace cloudserver.bucketquota

@http(method: "PUT", uri: "/{Bucket}?quota=true")
@idempotent
operation UpdateBucketQuota {
Copy link
Contributor

Choose a reason for hiding this comment

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

keep same conventions as AWS (and http/rest in general) ?

Suggested change
operation UpdateBucketQuota {
operation PutBucketQuota {

input := {
@required
@httpLabel
Bucket: String

@httpPayload
quota: String
}
output := {
@httpPayload
body: String
}
}
31 changes: 24 additions & 7 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,22 +1,38 @@
{
"name": "@scality/cloudserverclient",
"version": "1.0.0",
"version": "1.0.1",
"engines": {
"node": ">=20"
},
"description": "Smithy-generated TypeScript client for Cloudserver's internal APIs",
"main": "dist/index.js",
"types": "dist/index.d.ts",
"exports": {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will allow importing with subpath, for example

import { BucketQuotaClient } from '@scality/cloudserverclient/quota'

".": {
"types": "./dist/index.d.ts",
"default": "./dist/index.js"
},
"./*": {
"types": "./dist/*.d.ts",
"default": "./dist/*.js"
}
},
"files": [
"dist",
"build/smithy/source/typescript-codegen"
"build/smithy/cloudserver/typescript-codegen",
"build/smithy/cloudserverBucketQuota/typescript-codegen"
],
"publishConfig": {
"access": "public",
"registry": "https://registry.npmjs.org"
},
"scripts": {
"clean:build": "rm -rf build dist",
"build:smithy": "smithy build",
"build:generated": "cd build/smithy/source/typescript-codegen && yarn install && yarn build",
"build:generated": "cd build/smithy/cloudserver/typescript-codegen && yarn install && yarn build",
"build:generated:bucketQuota": "cd build/smithy/cloudserverBucketQuota/typescript-codegen && yarn install && yarn build",
"build:wrapper": "tsc",
"build": "yarn install && yarn clean:build && yarn build:smithy && yarn build:generated && yarn build:wrapper",
"build": "yarn install && yarn clean:build && yarn build:smithy && yarn build:generated && yarn build:generated:bucketQuota && yarn build:wrapper",
"test": "jest",
"test:indexes": "jest tests/testIndexesApis.test.ts",
"test:error-handling": "jest tests/testErrorHandling.test.ts",
Expand All @@ -25,14 +41,15 @@
"test:lifecycle": "jest tests/testLifecycleApis.test.ts",
"test:metadata": "jest tests/testMetadataApis.test.ts",
"test:raft": "jest tests/testRaftApis.test.ts",
"test:mongo-backend": "yarn test:indexes && yarn test:error-handling && yarn test:multiple-backend",
"test:bucketQuotas": "jest tests/testQuotaApis.test.ts",
"test:mongo-backend": "yarn test:indexes && yarn test:error-handling && yarn test:multiple-backend && yarn test:bucketQuotas",
"test:metadata-backend": "yarn test:api && yarn test:lifecycle && yarn test:metadata && yarn test:raft",
"lint": "eslint src tests",
"typecheck": "tsc --noEmit"
},
"devDependencies": {
"@eslint/compat": "^2.0.0",
"@scality/eslint-config-scality": "scality/Guidelines#8.3.0",
"@scality/eslint-config-scality": "github:scality/Guidelines#8.3.0",
"@types/jest": "^30.0.0",
"@typescript-eslint/eslint-plugin": "^6.21.0",
"@typescript-eslint/parser": "^6.21.0",
Expand All @@ -51,6 +68,6 @@
"license": "Apache-2.0",
"repository": {
"type": "git",
"url": "https://github.com/scality/cloudserverclient.git"
"url": "git+https://github.com/scality/cloudserverclient.git"
}
}
19 changes: 19 additions & 0 deletions service/cloudserverBucketQuota.smithy
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
$version: "2.0"

namespace cloudserver.bucketquota

use aws.protocols#restXml
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have to create a new separate service, as the other client is json, but the quota apis are xml.
And anyways it might just be better for code organisation.

use aws.auth#sigv4
use aws.api#service

@restXml(noErrorWrapping: true)
@sigv4(name: "s3")
@service(sdkId: "cloudserverBucketQuota")
service CloudserverBucketQuota {
version: "2018-07-11",
operations: [
GetBucketQuota,
UpdateBucketQuota,
DeleteBucketQuota,
]
}
22 changes: 18 additions & 4 deletions smithy-build.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,24 @@
"software.amazon.smithy.typescript:smithy-aws-typescript-codegen:0.34.0"
]
},
"plugins": {
"typescript-codegen": {
"package": "@scality/cloudserverclient",
"packageVersion": "1.0.0"
"projections": {
"cloudserver": {
"plugins": {
"typescript-codegen": {
"service": "cloudserver.client#cloudserver",
"package": "@scality/cloudserverclient",
"packageVersion": "1.0.0"
}
}
},
"cloudserverBucketQuota": {
"plugins": {
"typescript-codegen": {
"service": "cloudserver.bucketquota#CloudserverBucketQuota",
"package": "@scality/cloudserverclient-bucket",
"packageVersion": "1.0.0"
}
}
}
}
}
37 changes: 37 additions & 0 deletions src/clients/bucketQuota.ts

Choose a reason for hiding this comment

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

We can create a new folder client or rename these files with client. Quota.ts is not very friendly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had the same thought, but i thought it would look weird because the import path would end up with "client" twice. But still, I moved everything in a /clients folder it will be better

Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import {
CloudserverBucketQuota,
CloudserverBucketQuotaClientConfig,
GetBucketQuotaCommand,
GetBucketQuotaCommandOutput,
UpdateBucketQuotaCommand,
UpdateBucketQuotaCommandOutput,
DeleteBucketQuotaCommand,
DeleteBucketQuotaCommandOutput
} from '../../build/smithy/cloudserverBucketQuota/typescript-codegen';
import { CloudserverClientConfig } from '../../build/smithy/cloudserver/typescript-codegen';

export class BucketQuotaClient {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ended up creating a wrapper, it will be easier to use as you will see in the tests, and i was kinda forced to do it because of the update quota api. Basically we probably made a mistake when designing the update quota apis, which wants to receive a structure like this :

<quota>1000002</quota>
But ideally it should look like this :

<parent>
    <quota>1000002</quota>
    <otherProperties>
</parent>

When using the first structure, we can't use integers, so I was forced to send a string like this, but since we want a convenient api, I don't want frontend colleagues to have to call
client.updateQuota(bucket, '<quota>12</quota>')) which is ridiculous
so with this they will be able to send
client.updateQuota(bucket, 12'))

Copy link
Contributor

Choose a reason for hiding this comment

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

there is one subtelty regarding passing quota as "number", related to javascript:

  • javascript numbers are actually floats (as long as the number is small enough, they behave just like regular integers, but their true nature appears at some point) ;
  • then we got similar issue in JSON (not sure about XML), where large numbers must be passed as string, otherwise they are not properly de-serialized...

At production-scale, quotas are large : so we ended up having to "force" use of string at some places to ensure large quotas can be properly retrieved...
→ please make sure it still works
→ as "input", we should support both number and bigint

private client: CloudserverBucketQuota;
Copy link
Contributor

Choose a reason for hiding this comment

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

ok to introduce a wrapper (c.f. previous point) ; however, this specific wrapper changes the overall "form" of the API, which does not look like AWS sdk v3 code : looks more like v2.

I wonder if it would not be better to wrap the command objects (GetBucketQuotaCommand, UpdateBucketQuotaCommand...) then send them as usual?

(this could also make it more seamless to have combined "s3" + quotas client: just need to write our own client whose send() method supports both s3 & quota api ?)


constructor(config: CloudserverClientConfig | CloudserverBucketQuotaClientConfig) {
this.client = new CloudserverBucketQuota(config as CloudserverBucketQuotaClientConfig);
}

async getBucketQuota(bucketName: string): Promise<GetBucketQuotaCommandOutput> {
const command = new GetBucketQuotaCommand({ Bucket: bucketName });
return this.client.send(command);
}

async updateBucketQuota(bucketName: string, quota: number): Promise<UpdateBucketQuotaCommandOutput> {
const command = new UpdateBucketQuotaCommand({
Bucket: bucketName,
quota: `<quota>${quota}</quota>`,
Copy link
Contributor

Choose a reason for hiding this comment

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

seems weird to handle the XML here : should have a different definition of quota in the protocol?

i.e. something more complex instead of just:

       @httpPayload
        quota: String

Copy link
Contributor

Choose a reason for hiding this comment

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

note: not sure if this is possible and/or the only way, but this <quota>{int}</quota> is actually a corner case of a list of quota integers...

so maybe:

       @httpPayload
        quota: Quota

and

list Quota {
  member: string
}

would do it... (and we just need to verify there is a single entry in the list)

});
return this.client.send(command);
}

async deleteBucketQuota(bucketName: string): Promise<DeleteBucketQuotaCommandOutput> {
const command = new DeleteBucketQuotaCommand({ Bucket: bucketName });
return this.client.send(command);
}
}
18 changes: 18 additions & 0 deletions src/clients/cloudserver.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import {
Copy link
Contributor

Choose a reason for hiding this comment

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

so now moving to separate clients for each service?
→ in that case, this specific client must be renamed ("BackeatRoutesClient" ?), as it does not cover all cloudserver routes...

even if each client is available "separately", may be good to keep an "aggregated" cloudserver client:

  • for retro-compatibility, avoid breaking change (e.g. allow easily bumping in backbeat)
  • for ease of use & discoverability

CloudserverClient as GeneratedCloudserverClient,
CloudserverClientConfig
} from '../../build/smithy/cloudserver/typescript-codegen';
import { createCustomErrorMiddleware } from '../utils';

export * from '../../build/smithy/cloudserver/typescript-codegen';
export class CloudserverClient extends GeneratedCloudserverClient {
constructor(config: CloudserverClientConfig) {
super(config);

this.middlewareStack.add(createCustomErrorMiddleware(), {
step: 'deserialize',
name: 'cloudserverErrorHandler',
priority: 'normal',
});
}
}
21 changes: 2 additions & 19 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,3 @@
import {
CloudserverClient as GeneratedCloudserverClient,
CloudserverClientConfig
} from '../build/smithy/source/typescript-codegen';
import { createCustomErrorMiddleware } from './utils';

export * from '../build/smithy/source/typescript-codegen';
export * from './clients/cloudserver';
export { BucketQuotaClient } from './clients/bucketQuota';
export * from './utils';

export class CloudserverClient extends GeneratedCloudserverClient {
constructor(config: CloudserverClientConfig) {
super(config);

this.middlewareStack.add(createCustomErrorMiddleware(), {
step: 'deserialize',
name: 'cloudserverErrorHandler',
priority: 'normal',
});
}
}
2 changes: 1 addition & 1 deletion src/utils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { XMLParser } from 'fast-xml-parser';
import { CloudserverServiceException } from '../build/smithy/source/typescript-codegen';
import { CloudserverServiceException } from '../build/smithy/cloudserver/typescript-codegen';

/**
* Adds middleware to manually set the Content-Length header on a command.
Expand All @@ -18,14 +18,14 @@
return;
}

const commandWithMiddleware = command as any;

Check warning on line 21 in src/utils.ts

View workflow job for this annotation

GitHub Actions / Lint and typecheck

Unexpected any. Specify a different type
if (!commandWithMiddleware.middlewareStack) {
throw new Error('Command does not have a middleware stack');
}

commandWithMiddleware.middlewareStack.add(
(next: any) => async (args: any) => {

Check warning on line 27 in src/utils.ts

View workflow job for this annotation

GitHub Actions / Lint and typecheck

Unexpected any. Specify a different type

Check warning on line 27 in src/utils.ts

View workflow job for this annotation

GitHub Actions / Lint and typecheck

Unexpected any. Specify a different type
const request = args.request as any;

Check warning on line 28 in src/utils.ts

View workflow job for this annotation

GitHub Actions / Lint and typecheck

Unexpected any. Specify a different type
if (request?.headers && !request.headers['content-length']) {
request.headers['content-length'] = String(contentLength);
}
Expand All @@ -38,10 +38,10 @@
}

export function createCustomErrorMiddleware() {
return (next: any) => async (args: any) => {

Check warning on line 41 in src/utils.ts

View workflow job for this annotation

GitHub Actions / Lint and typecheck

Unexpected any. Specify a different type

Check warning on line 41 in src/utils.ts

View workflow job for this annotation

GitHub Actions / Lint and typecheck

Unexpected any. Specify a different type
try {
return await next(args);
} catch (error: any) {

Check warning on line 44 in src/utils.ts

View workflow job for this annotation

GitHub Actions / Lint and typecheck

Unexpected any. Specify a different type
const parseXmlError = (xml: string) => {
try {
const result = new XMLParser({}).parse(xml);
Expand All @@ -68,7 +68,7 @@
const xml = body?.toString() || '';
const errorInfo = parseXmlError(xml);

const xmlError: any = new CloudserverServiceException({

Check warning on line 71 in src/utils.ts

View workflow job for this annotation

GitHub Actions / Lint and typecheck

Unexpected any. Specify a different type
name: errorInfo.code || error.name,
message: errorInfo.message || 'XML error response',
$fault: statusCode >= 500 ? 'server' : 'client',
Expand All @@ -88,7 +88,7 @@
const title = html.match(/<title[^>]*>([^<]+)<\/title>/i);
const message = title && title[1] || 'HTML error response';

const htmlError: any = new CloudserverServiceException({

Check warning on line 91 in src/utils.ts

View workflow job for this annotation

GitHub Actions / Lint and typecheck

Unexpected any. Specify a different type
name: `HTML ${response?.reason || 'Error'}`,
message,
$fault: statusCode >= 500 ? 'server' : 'client',
Expand Down
Loading
Loading