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

Remove Metadata length constraint and update dependencies #219

Merged
merged 3 commits into from
Oct 13, 2023
Merged
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
4 changes: 2 additions & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#
# Build the application
#
FROM registry.access.redhat.com/ubi9/nodejs-18:1-62.1692771036 as builder
FROM registry.access.redhat.com/ubi9/nodejs-18:1-70.1695740477 as builder

ENV NO_UPDATE_NOTIFIER=true

Expand All @@ -22,7 +22,7 @@ RUN npm ci --omit=dev
#
# Create the final container image
#
FROM registry.access.redhat.com/ubi9/nodejs-18-minimal:1-67
FROM registry.access.redhat.com/ubi9/nodejs-18-minimal:1-74.1695740475

ENV APP_PORT=3000 \
NO_UPDATE_NOTIFIER=true
Expand Down
7,741 changes: 4,281 additions & 3,460 deletions app/package-lock.json

Large diffs are not rendered by default.

20 changes: 10 additions & 10 deletions app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@
"seed": "knex seed:run"
},
"dependencies": {
"@aws-sdk/client-s3": "^3.388.0",
"@aws-sdk/lib-storage": "^3.388.0",
"@aws-sdk/s3-request-presigner": "^3.388.0",
"api-problem": "^9.0.1",
"@aws-sdk/client-s3": "^3.423.0",
"@aws-sdk/lib-storage": "^3.423.0",
"@aws-sdk/s3-request-presigner": "^3.423.0",
"api-problem": "^9.0.2",
"compression": "^1.7.4",
"config": "^3.3.9",
"content-disposition": "^0.5.4",
Expand All @@ -41,22 +41,22 @@
"express": "^4.18.2",
"express-basic-auth": "^1.2.1",
"express-winston": "^4.2.0",
"joi": "^17.10.2",
"joi": "^17.11.0",
"js-yaml": "^4.1.0",
"jsonwebtoken": "^9.0.1",
"jsonwebtoken": "^9.0.2",
"knex": "^2.5.1",
"objection": "^3.1.1",
"pg": "^8.11.2",
"objection": "^3.1.2",
"pg": "^8.11.3",
"winston": "^3.10.0",
"winston-transport": "^4.5.0"
},
"devDependencies": {
"aws-sdk-client-mock": "^3.0.0",
"aws-sdk-client-mock-jest": "^3.0.0",
"eslint": "^8.47.0",
"eslint": "^8.50.0",
"eslint-config-recommended": "^4.1.0",
"eslint-plugin-prettier": "^5.0.0",
"jest": "~29.3.1",
"jest": "^29.7.0",
"jest-joi": "^1.1.17",
"nodemon": "^3.0.1",
"supertest": "^6.3.3"
Expand Down
3 changes: 2 additions & 1 deletion app/src/components/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -277,14 +277,15 @@ const utils = {

/**
* @function isAtPath
* Predicate function determining if the `path` is a member of the `prefix` path
* Predicate function determining if the `path` is a non-directory member of the `prefix` path
* @param {string} prefix The base "folder"
* @param {string} path The "file" to check
* @returns {boolean} True if path is member of prefix. False in all other cases.
*/
isAtPath(prefix, path) {
if (typeof prefix !== 'string' || typeof path !== 'string') return false;
if (prefix === path) return true; // Matching strings are always at the at the path
if (path.endsWith(DELIMITER)) return false; // Trailing slashes references the folder

const pathParts = path.split(DELIMITER).filter(part => part);
const prefixParts = prefix.split(DELIMITER).filter(part => part);
Expand Down
17 changes: 17 additions & 0 deletions app/src/db/migrations/20231010000000_011-metadata-text.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
exports.up = function (knex) {
return Promise.resolve()
// Change to text type
.then(() => knex.schema.alterTable('metadata', table => {
table.text('key').notNullable().alter();
table.text('value').notNullable().alter();
}));
};

exports.down = function (knex) {
return Promise.resolve()
// Revert back to varchar(255) type
.then(() => knex.schema.alterTable('metadata', table => {
table.string('key', 255).notNullable().alter();
table.string('value', 255).notNullable().alter();
}));
};
4 changes: 2 additions & 2 deletions app/src/db/models/tables/metadata.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ class Metadata extends Model {
required: ['key', 'value'],
properties: {
id: { type: 'integer' },
key: { type: 'string', minLength: 1, maxLength: 255 },
value: { type: 'string', minLength: 1, maxLength: 255 }
key: { type: 'string', minLength: 1 },
value: { type: 'string', minLength: 1 }
},
additionalProperties: false
};
Expand Down
4 changes: 2 additions & 2 deletions app/src/docs/v1.api-spec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1477,10 +1477,10 @@ components:
description: >-
An arbitrary metadata key/value pair. Must contain the x-amz-meta-
prefix to be valid. Multiple metadata pairs can be defined. Keys must be
unique and will be converted to lowercase.
unique, contain no whitespace, and will be converted to lowercase.
schema:
type: string
maximum: 255
minimum: 1
example:
- x-amz-meta-foo
- x-amz-meta-bar
Expand Down
4 changes: 3 additions & 1 deletion app/src/validators/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,9 @@ const type = {
* @param {number} [options.minValueStringLength=1] Optional minimum string length of metadata value allowed,
* @returns {object} Joi object
*/
// TODO: Simplify by changing from arrow function to property
metadata: ({ minKeyCount = 0, minValueStringLength = 1 } = {}) => Joi.object()
.pattern(/^x-amz-meta-.{1,255}$/i, Joi.string().min(minValueStringLength).max(255), { matches: Joi.array().min(minKeyCount) })
.pattern(/^x-amz-meta-\S+$/i, Joi.string().min(minValueStringLength), { matches: Joi.array().min(minKeyCount) })
.unknown(),

/**
Expand All @@ -63,6 +64,7 @@ const type = {
* (default of 9 because COMS also adds a `coms-id` tag by default)
* @returns {object} Joi object
*/
// TODO: Simplify by changing from arrow function to property
tagset: ({ maxKeyCount = 9, minKeyCount = 0, minValueStringLength = 0 } = {}) => Joi.object()
.pattern(
/^(?!coms-id$).{1,255}$/, // don't allow key 'coms-id'
Expand Down
2 changes: 2 additions & 0 deletions app/tests/unit/components/utils.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -396,9 +396,11 @@ describe('isAtPath', () => {
[true, '/foo', 'foo/bar'],
[true, '/foo', '/foo/bar'],
[true, 'a/b', 'a/b/foo.jpg'],
[false, 'a', 'a/b/'], // Trailing slashes references the folder and should be excluded
[false, 'a/b', 'a/b/'], // Trailing slashes references the folder and should be excluded
[false, 'a/b', 'a/b/z/deep.jpg'],
[false, 'a/b', 'a/b/y/z/deep.jpg'],
[false, 'a/b', 'a/b/c/'], // Trailing slashes references the folder and should be excluded
[false, 'a/b/c', 'a/b/c/'], // Trailing slashes references the folder and should be excluded
[false, 'a/b/c', 'a/bar.png'],
[false, 'c/b/a', 'a/b/c/bar.png'],
Expand Down
8 changes: 1 addition & 7 deletions app/tests/unit/validators/common.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ describe('type', () => {
it('enforces general metadata pattern', () => {
expect(model.patterns).toEqual(expect.arrayContaining([
expect.objectContaining({
regex: '/^x-amz-meta-.{1,255}$/i',
regex: '/^x-amz-meta-\\S+$/i',
rule: expect.objectContaining({
type: 'string',
rules: expect.arrayContaining([
Expand All @@ -156,12 +156,6 @@ describe('type', () => {
args: expect.objectContaining({
limit: 1
})
}),
expect.objectContaining({
name: 'max',
args: expect.objectContaining({
limit: 255
})
})
])
})
Expand Down
8 changes: 1 addition & 7 deletions app/tests/unit/validators/object.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ describe('searchObjects', () => {
it('enforces general metadata pattern', () => {
expect(headers.patterns).toEqual(expect.arrayContaining([
expect.objectContaining({
regex: '/^x-amz-meta-.{1,255}$/i',
regex: '/^x-amz-meta-\\S+$/i',
rule: expect.objectContaining({
type: 'string',
rules: expect.arrayContaining([
Expand All @@ -400,12 +400,6 @@ describe('searchObjects', () => {
args: expect.objectContaining({
limit: 1
})
}),
expect.objectContaining({
name: 'max',
args: expect.objectContaining({
limit: 255
})
})
])
})
Expand Down