Skip to content

feat: sign record's public url #358

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

Merged
merged 14 commits into from
Apr 1, 2025
Merged
Show file tree
Hide file tree
Changes from 7 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
553 changes: 401 additions & 152 deletions package-lock.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
"@typescript-eslint/eslint-plugin": "^7.0.0",
"@typescript-eslint/parser": "^7.0.0",
"commitlint": "^19.3.0",
"crawlee": "^3.11.5",
"crawlee": "^3.13",
Copy link
Member

Choose a reason for hiding this comment

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

please keep the full version definition here

"eslint": "^8.57.0",
"fs-extra": "^11.2.0",
"gen-esm-wrapper": "^1.1.3",
Expand Down
12 changes: 6 additions & 6 deletions packages/apify/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,15 @@
"@apify/input_secrets": "^1.1.40",
"@apify/log": "^2.4.3",
"@apify/timeout": "^0.3.0",
"@apify/utilities": "^2.9.3",
"@crawlee/core": "^3.9.0",
"@crawlee/types": "^3.9.0",
"@crawlee/utils": "^3.9.0",
"apify-client": "^2.12.0",
"@apify/utilities": "^2.13.0",
"@crawlee/core": "^3.12.3-beta.12",
"@crawlee/types": "^3.12.3-beta.12",
"@crawlee/utils": "^3.12.3-beta.12",
Copy link
Member

Choose a reason for hiding this comment

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

and don't forget to bump this

"apify-client": "^2.11.1",
Copy link
Member

Choose a reason for hiding this comment

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

why the downgrade here? 👀

"fs-extra": "^11.2.0",
"ow": "^0.28.2",
"semver": "^7.5.4",
"tslib": "^2.6.2",
"ws": "^8.18.0"
}
}
}
3 changes: 3 additions & 0 deletions packages/apify/src/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export interface ConfigurationOptions extends CoreConfigurationOptions {
actorRunId?: string;
actorTaskId?: string;
apiBaseUrl?: string;
apiPublicBaseUrl?: string;
Copy link
Member

Choose a reason for hiding this comment

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

lets add a comment here about the difference between apiBaseUrl and apiPublicBaseUrl

containerPort?: number;
containerUrl?: string;
proxyHostname?: string;
Expand Down Expand Up @@ -139,6 +140,7 @@ export class Configuration extends CoreConfiguration {
APIFY_ACTOR_EVENTS_WS_URL: 'actorEventsWsUrl',
APIFY_ACTOR_ID: 'actorId',
APIFY_API_BASE_URL: 'apiBaseUrl',
APIFY_API_PUBLIC_BASE_URL: 'apiPublicBaseUrl',
APIFY_IS_AT_HOME: 'isAtHome',
APIFY_ACTOR_RUN_ID: 'actorRunId',
APIFY_ACTOR_TASK_ID: 'actorTaskId',
Expand Down Expand Up @@ -183,6 +185,7 @@ export class Configuration extends CoreConfiguration {
defaultRequestQueueId: LOCAL_ACTOR_ENV_VARS[ACTOR_ENV_VARS.DEFAULT_REQUEST_QUEUE_ID],
inputKey: 'INPUT',
apiBaseUrl: 'https://api.apify.com',
apiPublicBaseUrl: 'https://api.apify.com',
proxyStatusUrl: 'http://proxy.apify.com',
proxyHostname: LOCAL_APIFY_ENV_VARS[APIFY_ENV_VARS.PROXY_HOSTNAME],
proxyPort: +LOCAL_APIFY_ENV_VARS[APIFY_ENV_VARS.PROXY_PORT],
Expand Down
12 changes: 10 additions & 2 deletions packages/apify/src/key_value_store.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { createHmacSignature } from '@apify/utilities';
import type { StorageManagerOptions } from '@crawlee/core';
import { KeyValueStore as CoreKeyValueStore } from '@crawlee/core';

Expand All @@ -15,11 +16,18 @@ export class KeyValueStore extends CoreKeyValueStore {
* access the value in the remote key-value store.
*/
override getPublicUrl(key: string): string {
if (!(this.config as Configuration).get('isAtHome') && getPublicUrl) {
const config = this.config as Configuration;
if (!config.get('isAtHome') && getPublicUrl) {
return getPublicUrl.call(this, key);
}

return `https://api.apify.com/v2/key-value-stores/${this.id}/records/${key}`;
const publicUrl = new URL(`${config.get('apiPublicBaseUrl', 'https://api.apify.com')}/v2/key-value-stores/${this.id}/records/${key}`);
Copy link
Member

Choose a reason for hiding this comment

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

no need for the explicit default here, its the exact same value as you define on the config level


if (this.storageObject?.urlSigningSecretKey) {
publicUrl.searchParams.append('signature', createHmacSignature(this.storageObject.urlSigningSecretKey as string, key));
}

return publicUrl.toString();
}

/**
Expand Down
21 changes: 21 additions & 0 deletions test/e2e/sdk/publicUrl/src/main.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { Actor, log } from 'apify';

// Also needs to be changed in test.mjs
const PUBLIC_RECORD_KEY = 'public-record-key';
Copy link

@tobice tobice Mar 17, 2025

Choose a reason for hiding this comment

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

(opt) How about you pass this as input to the Actor? All three values...

  • data to upload
  • key where to upload the data
  • key where to store the public URL

Then you don't need to define anything in two places and the test will be more readable (not that it is super complicated or anything 😄 )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point, thank you 🙏 Refactored it

const PUBLIC_DATA = { exposedData: 'test' };

await Actor.init();

await Actor.setValue(PUBLIC_RECORD_KEY, JSON.stringify(PUBLIC_DATA), { contentType: `application/json` });

const defaultKeyValueStore = await Actor.openKeyValueStore();
const publicUrl = defaultKeyValueStore.getPublicUrl(PUBLIC_RECORD_KEY);

// Here we store the url itself
await Actor.setValue('urlToPublicData', publicUrl);

log.info('Generated public url', { publicUrl });

await Actor.pushData({ publicUrl });

await Actor.exit();
20 changes: 20 additions & 0 deletions test/e2e/sdk/publicUrl/test.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import assert from 'node:assert/strict';

import { ApifyClient } from 'apify';

// Also needs to be changed in main.mjs
const PUBLIC_DATA = { exposedData: 'test' };

const client = new ApifyClient({
token: process.env.APIFY_TOKEN,
});

const actor = client.actor(process.argv[2]);

const run = await actor.call({}, { waitSecs: 15 });
assert.equal(run.exitCode, 0);

const publicUrl = await client.keyValueStore(run.defaultKeyValueStoreId).getRecord('urlToPublicData');
const data = await fetch(publicUrl.value).then((res) => res.json());

assert.deepEqual(data, PUBLIC_DATA);
Loading