Make accessKey and secretKey optional and add support to use IRSA Iam role #1716

Anany-k · 2025-07-21T06:34:36Z

Context

Currently accesskey and secretkey are required config needed for integration with Object Storage

Proposed solution

Minio supports IRSA EKS role already.

Related issues

Has this been tested?

👍 yes, I added tests to the test suite
💭 no, because this PR is a draft and still needs work
🙅 no, because this is not relevant here
🙋 no, because I need help
I have only tested this when running on EKS configured with IRSA role. Will need guidance to make this MR mergeable.

Screenshots / Screencasts

… role

github-actions · 2025-07-21T06:34:46Z

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

fflorent

Hello,

Thank you for your contribution

It sounds like there exist tests that are failing after your work.

This documentation for developers is a guide to help you, take a look and feel free to ask questions (we think there are rooms for improvements)

  13) UsersManager
       multi-worker
         "before all" hook in "multi-worker":
     Error: Unable to get credentials: Error: Failed to get Credentials: TypeError [ERR_INVALID_CHAR]: Invalid character in header content ["X-aws-ec2-metadata-token"]
      at Client.checkAndRefreshCreds (node_modules/minio/dist/main/internal/client.ts:541:15)
      at processTicksAndRejections (node:internal/process/task_queues:105:5)
      at Client.makeRequestStreamAsync (node_modules/minio/dist/main/internal/client.ts:706:5)
      at Client.getBucketVersioning (node_modules/minio/dist/main/internal/client.ts:2159:21)
      at MinIOExternalStorage.hasVersioning (app/server/lib/MinIOExternalStorage.ts:164:24)
      at checkMinIOBucket (app/server/lib/configureMinIOExternalStorage.ts:74:8)
      at CoreCreate.checkBackend (app/server/lib/ICreate.ts:176:9)
      at FlexServer.addDoc (app/server/lib/FlexServer.ts:1463:7)
      at MergedServer.run (app/server/MergedServer.ts:198:9)
      at MergedServer.run (app/server/MergedServer.ts:220:11)
      at Context.<anonymous> (test/gen-server/lib/homedb/UsersManager.ts:1220:7)
     Caused by: Error: Failed to get Credentials: TypeError [ERR_INVALID_CHAR]: Invalid character in header content ["X-aws-ec2-metadata-token"]
      at IamAwsProvider.fetchCredentials (node_modules/minio/dist/main/IamAwsProvider.ts:91:13)
      at processTicksAndRejections (node:internal/process/task_queues:105:5)
      at IamAwsProvider.getCredentials (node_modules/minio/dist/main/IamAwsProvider.ts:60:27)
      at Client.checkAndRefreshCreds (node_modules/minio/dist/main/internal/client.ts:536:33)
      at Client.makeRequestStreamAsync (node_modules/minio/dist/main/internal/client.ts:706:5)
      at Client.getBucketVersioning (node_modules/minio/dist/main/internal/client.ts:2159:21)
      at MinIOExternalStorage.hasVersioning (app/server/lib/MinIOExternalStorage.ts:164:24)
      at checkMinIOBucket (app/server/lib/configureMinIOExternalStorage.ts:74:8)
      at CoreCreate.checkBackend (app/server/lib/ICreate.ts:176:9)
      at FlexServer.addDoc (app/server/lib/FlexServer.ts:1463:7)
      at MergedServer.run (app/server/MergedServer.ts:198:9)
      at MergedServer.run (app/server/MergedServer.ts:220:11)
      at Context.<anonymous> (test/gen-server/lib/homedb/UsersManager.ts:1220:7)
     Caused by: TypeError: Invalid character in header content ["X-aws-ec2-metadata-token"]
      at ClientRequest.setHeader (node:_http_outgoing:703:3)
      at new ClientRequest (node:_http_client:302:14)
      at Object.request (node:http:102:10)
      at /home/runner/work/grist-core/grist-core/node_modules/minio/dist/main/internal/request.ts:17:34
      at new Promise (<anonymous>)
      at request (node_modules/minio/dist/main/internal/request.ts:16:10)
      at IamAwsProvider.getIamRoleName (node_modules/minio/dist/main/IamAwsProvider.ts:184:30)
      at IamAwsProvider.getIamRoleNamedUrl (node_modules/minio/dist/main/IamAwsProvider.ts:166:33)
      at IamAwsProvider.fetchCredentials (node_modules/minio/dist/main/IamAwsProvider.ts:86:26)
      at processTicksAndRejections (node:internal/process/task_queues:105:5)
      at IamAwsProvider.getCredentials (node_modules/minio/dist/main/IamAwsProvider.ts:60:27)
      at Client.checkAndRefreshCreds (node_modules/minio/dist/main/internal/client.ts:536:33)
      at Client.makeRequestStreamAsync (node_modules/minio/dist/main/internal/client.ts:706:5)
      at Client.getBucketVersioning (node_modules/minio/dist/main/internal/client.ts:2159:21)
      at MinIOExternalStorage.hasVersioning (app/server/lib/MinIOExternalStorage.ts:164:24)
      at checkMinIOBucket (app/server/lib/configureMinIOExternalStorage.ts:74:8)
      at CoreCreate.checkBackend (app/server/lib/ICreate.ts:176:9)
      at FlexServer.addDoc (app/server/lib/FlexServer.ts:1463:7)
      at MergedServer.run (app/server/MergedServer.ts:198:9)
      at MergedServer.run (app/server/MergedServer.ts:220:11)
      at Context.<anonymous> (test/gen-server/lib/homedb/UsersManager.ts:1220:7)

fflorent · 2025-07-21T07:19:53Z

app/server/lib/configureMinIOExternalStorage.ts

 import {wrapWithKeyMappedStorage} from 'app/server/lib/ExternalStorage';
 import {appSettings} from 'app/server/lib/AppSettings';
 import {MinIOExternalStorage} from 'app/server/lib/MinIOExternalStorage';
+import {IamAwsProvider} from 'minio/dist/main/IamAwsProvider.js';


Nit: please remove the .js extension

Suggested change

import {IamAwsProvider} from 'minio/dist/main/IamAwsProvider.js';

import {IamAwsProvider} from 'minio/dist/main/IamAwsProvider';

fflorent · 2025-07-21T07:20:49Z

app/server/lib/MinIOExternalStorage.ts

-      secretKey: string,
+      accessKey?: string,
+      secretKey?: string,
+      credentialsProvider?: any,


Could you type this property?

Import IamAwsProvider like in app/server/lib/configureMinIOExternalStorage.ts and then:

Suggested change

credentialsProvider?: any,

credentialsProvider?: IamAwsProvider,

fflorent · 2025-07-21T07:22:13Z

app/server/lib/configureMinIOExternalStorage.ts

    envVar: ['GRIST_DOCS_MINIO_SECRET_KEY'],
    censor: true,
  });
+  const credentialsProvider = new IamAwsProvider({});


What happens if we want to use accessKey + secretKey and not IRSA?

Anany-k · 2025-07-21T09:55:52Z

I have read the CLA Document and I hereby sign the CLA

fflorent · 2025-07-21T12:41:57Z

app/server/lib/MinIOExternalStorage.ts

  ) {
+    if (!options.accessKey || !options.secretKey) {
+      options.credentialsProvider = new IamAwsProvider({});
+    }


I imagine an administrator having written a typo in the env variables for either the access key and the secret key.

I think they would be confused by having switched implicitly to the IRSA authentication method.

Also it seems that it is not the only alternative to exist to authenticate with MinIO, currently there also exists AssumeRoleProvider (I don't ask to implement it, if people ask for supporting this, then it should be in another future PR).

What do you think of something like the following instead:

introduce an env variable, like GRIST_DOCS_MINIO_AUTH_TYPE (or another meaningful name) that would either equal to credentials or to IRSA;

by default, GRIST_DOCS_MINIO_AUTH_TYPE equals to credentials for the sake of backward compatibility;

if GRIST_DOCS_MINIO_AUTH_TYPE equals to credentials, the env variables GRIST_DOCS_MINIO_ACCESS_KEY and GRIST_DOCS_MINIO_SECRET_KEY are mandatory in configureMinIOExternalStorage.ts. Otherwise these values are not read.

Also you may indeed instantiate directly a CredentialProvider or a IamAwsProvider directly in configureMinIOExternalStorage.ts and pass it as the credentialsProvider property to this class (and remove the accessKey and secretKey from the options here).

I think it probably should be similar to official sdk that just goes through the credential provider chain without having to explicitly specify what method we wanna use.

I believe the minio client kind of does that here.

I see. Therefore I think you should probably detect the presence of the env variables that should be set for the IRSA, and if they are missing, then switch back to the classic auth method with credentials.

Please note that I am not an authority on this project, I'll let Grist Labs review your work on their side.

I added a env var GRIST_DOCS_MINIO_AUTH_AUTO_DETECT to try and load the credentials according to credential provider chain if GRIST_DOCS_MINIO_AUTH_AUTO_DETECT: true

…oading cred for IRSA

Anany-k · 2025-09-01T16:10:49Z

@paulfitz Requesting a review for this PR.
Thanks

paulfitz · 2025-09-05T16:34:53Z

test/server/lib/MinIOExternalStorage.ts

+    ) {
+      super(bucket, options, batchSize);
+      if (mockClient) {
+        (this as any)._s3 = mockClient;


How does this differ from what MinIOExternalStorage already does?

Modified the MinIOExternalStorage constructor a bit, so we don't need this

paulfitz · 2025-09-05T16:40:54Z

app/server/lib/MinIOExternalStorage.ts

 import * as fse from 'fs-extra';
 import * as minio from 'minio';
 import * as stream from 'node:stream';
+import {IamAwsProvider} from 'minio/dist/main/IamAwsProvider.js';


imports should be ordered alphabetically. The ".js" presumably isn't needed? Also if the import could be just from 'minio' that would be more consistent than reaching into the specific package layout (minio/dist/main). But sometimes packages are wonky, so, fine if unavoidable.

(Not a js/ts expert)
I was not able to import from minio directly, looks like it does not export IamAwsProvider.
Also, when remove .js and just use import {IamAwsProvider} from 'minio/dist/main/IamAwsProvider; I get the below error.

node:internal/modules/cjs/loader:657 throw e; ^ Error: Cannot find module '/grist/node_modules/minio/dist/main/IamAwsProvider' at createEsmNotFoundErr (node:internal/modules/cjs/loader:1419:15) at finalizeEsmResolution (node:internal/modules/cjs/loader:1408:15) at resolveExports (node:internal/modules/cjs/loader:650:14) at Function._findPath (node:internal/modules/cjs/loader:717:31) at Function._resolveFilename (node:internal/modules/cjs/loader:1369:27) at defaultResolveImpl (node:internal/modules/cjs/loader:1025:19) at resolveForCJSWithHooks (node:internal/modules/cjs/loader:1030:22) at Function._load (node:internal/modules/cjs/loader:1192:37) at TracingChannel.traceSync (node:diagnostics_channel:322:14) at wrapModuleLoad (node:internal/modules/cjs/loader:237:24) at Module.require (node:internal/modules/cjs/loader:1463:12) at require (node:internal/modules/helpers:147:16) at Object.<anonymous> (/grist/_build/app/server/lib/MinIOExternalStorage.js:39:26) at Module._compile (node:internal/modules/cjs/loader:1706:14) at Object..js (node:internal/modules/cjs/loader:1839:10) at Module.load (node:internal/modules/cjs/loader:1441:32) { code: 'MODULE_NOT_FOUND', path: '/grist/node_modules/minio' } Node.js v22.19.0

paulfitz · 2025-09-05T16:42:44Z

app/server/lib/MinIOExternalStorage.ts

+    private _batchSize?: number
  ) {
+    if (options.authAutoDetect) {
+      options.credentialsProvider = new IamAwsProvider({});


How does this work? If the id/secret are set, and autodetect is also set, will the id/secret be used?

Looks like if autodetect is set, id/secret will be ignored.
It's due to how minio-js works right now. ref code.

Initially, my thought was to not have authAutoDetect flag.
If id/secret env vars are not set we can by default try and load IRSA creds.

Let me know your thoughts.
Thanks

Copilot

Pull Request Overview

This PR adds support for using IRSA (IAM Role for Service Accounts) authentication with MinIO storage, making accessKey and secretKey optional when running on EKS with appropriate IAM role configuration.

Makes accessKey and secretKey optional in MinIO configuration
Adds authAutoDetect flag to enable IRSA authentication via IamAwsProvider
Updates test infrastructure to support mocking of MinIO clients

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
configureMinIOExternalStorage.ts	Changes accessKey/secretKey from required to optional and adds authAutoDetect configuration flag
MinIOExternalStorage.ts	Refactors constructor to support optional credentials and IRSA authentication with IamAwsProvider
test/server/lib/MinIOExternalStorage.ts	Creates TestMinIOExternalStorage class to enable dependency injection for testing

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-05T16:43:49Z

app/server/lib/MinIOExternalStorage.ts

-    private _batchSize?: number,
-    private _s3 = new minio.Client(options) as unknown as MinIOClient
+    private _batchSize?: number
  ) {


Missing validation to ensure that either credentials (accessKey/secretKey) or authAutoDetect is provided. The code should validate that at least one authentication method is configured to prevent runtime failures.

Suggested change

) {

) {

// Validate that at least one authentication method is configured

const hasAccessKeys = Boolean(options.accessKey && options.secretKey);

if (!hasAccessKeys && !options.authAutoDetect) {

throw new ApiError('MinIOExternalStorage requires either accessKey/secretKey or authAutoDetect to be set for authentication.', 400);

}

Copilot · 2025-09-05T16:43:50Z

app/server/lib/configureMinIOExternalStorage.ts

+  const accessKey = settings.flag('accessKey').readString({
    envVar: ['GRIST_DOCS_MINIO_ACCESS_KEY'],
    censor: true,
  });
-  const secretKey = settings.flag('secretKey').requireString({
+  const secretKey = settings.flag('secretKey').readString({
    envVar: ['GRIST_DOCS_MINIO_SECRET_KEY'],
    censor: true,
  });


Since accessKey and secretKey are now optional, there should be validation logic to ensure that when authAutoDetect is false, both accessKey and secretKey are provided. Currently, the function could return undefined values for these required credentials.

Make accessKey and secretKey optional and add support to use IRSA Iam…

0a9223b

… role

fflorent reviewed Jul 21, 2025

View reviewed changes

Add support for IRSA role with object store

0733671

github-actions bot added a commit that referenced this pull request Jul 21, 2025

@Anany-k has signed the CLA in #1716

479c2d5

Anany-k marked this pull request as draft July 21, 2025 09:58

Fix test

2c72299

Anany-k marked this pull request as ready for review July 21, 2025 11:36

Anany-k added 2 commits July 21, 2025 20:38

Remove .js extension when importing

bf8b27d

revert

ae6b38b

fflorent reviewed Jul 21, 2025

View reviewed changes

Anany-k added 2 commits August 27, 2025 23:32

Merge branch 'gristlabs:main' into main

331781d

Add an env var GRIST_DOCS_MINIO_AUTH_AUTO_DETECT to when should try l…

8d10813

…oading cred for IRSA

Anany-k requested a review from fflorent August 28, 2025 00:33

paulfitz reviewed Sep 5, 2025

View reviewed changes

paulfitz requested a review from Copilot September 5, 2025 16:43

Copilot AI reviewed Sep 5, 2025

View reviewed changes

Anany-k added 4 commits September 6, 2025 03:14

Merge branch 'gristlabs:main' into main

157f687

Merge branch 'gristlabs:main' into main

5161271

Update how minio client is initliazed in the constructor

94c9c8a

Fix eslint warning

54b36f8

Anany-k requested a review from paulfitz September 7, 2025 06:35

Anany-k added 2 commits September 29, 2025 09:24

Merge branch 'gristlabs:main' into main

600374a

Merge branch 'gristlabs:main' into main

350e181

	import {IamAwsProvider} from 'minio/dist/main/IamAwsProvider.js';
	import {IamAwsProvider} from 'minio/dist/main/IamAwsProvider';

	credentialsProvider?: any,
	credentialsProvider?: IamAwsProvider,

-  ) {
+  ) {
+    // Validate that at least one authentication method is configured
+    const hasAccessKeys = Boolean(options.accessKey && options.secretKey);
+    if (!hasAccessKeys && !options.authAutoDetect) {
+      throw new ApiError('MinIOExternalStorage requires either accessKey/secretKey or authAutoDetect to be set for authentication.', 400);
+    }

Uh oh!

Make accessKey and secretKey optional and add support to use IRSA Iam role #1716

Are you sure you want to change the base?

Make accessKey and secretKey optional and add support to use IRSA Iam role #1716

Conversation

Anany-k commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

Proposed solution

Related issues

Has this been tested?

Screenshots / Screencasts

Uh oh!

github-actions bot commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fflorent left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Anany-k commented Jul 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Anany-k Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Anany-k commented Sep 1, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Anany-k Sep 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Anany-k commented Jul 21, 2025 •

edited

Loading

github-actions bot commented Jul 21, 2025 •

edited

Loading

Anany-k Aug 27, 2025 •

edited

Loading

Anany-k Sep 7, 2025 •

edited

Loading