Fix Helm chart does not support special characters in access/secret key #15243

dormanze · 2022-07-07T00:38:52Z

Description

Motivation and Context

Fix Helm chart does not support special characters in access/secret key.

How to test this PR?

prefabricate users with access/secretkey containing special characters.
like:

users:
  - accessKey: "tv:(()KW5$`>@/c:dB6_=.4|{jv?@!"
    secretKey: ".B_ES205}Mjzrh}G{emJi'LEuUE#:R"
    policy: consoleAdmin

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Optimization (provides speedup with no functional changes)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

Fixes a regression (If yes, please add commit-id or PR # here)
Documentation updated
Unit tests added/updated

Add two parameters to prevent shell execution failure caused by special characters

harshavardhana · 2022-07-07T00:40:10Z

@dormanze do not see any files changed ?

…_helm

dormanze · 2022-07-07T00:43:42Z

@dormanze do not see any files changed ?
Sorry, there is something wrong with my Internet.

upgrade comments.

harshavardhana

This change is too complicated what is the issue with helm chart not supporting these characters?

dormanze · 2022-07-07T04:23:04Z

This change is too complicated what is the issue with helm chart not supporting these characters?
Thank you for your attention.
It has nothing to do with helm.
In the shell, characters like {,',< will be parsed as keywords, so it will cause error messages.
You can create users by executing the following commands in the shell.

#accessKey is : tv:(()KW5$`>@/c:dB6_=.4|{jv?@!
#secretKey is : .B_ES205}Mjzrh}G{emJi'LEuUE#:R
mc admin user add myminio tv:(()KW5$`>@/c:dB6_=.4|{jv?@! .B_ES205}Mjzrh}G{emJi'LEuUE#:R --insecure

harshavardhana · 2022-07-07T05:24:42Z

#accessKey is : tv:(()KW5$>@/c:dB6_=.4|{jv?@! #secretKey is : .B_ES205}Mjzrh}G{emJi'LEuUE#:R mc admin user add myminio tv:(()KW5$>@/c:dB6_=.4|{jv?@! .B_ES205}Mjzrh}G{emJi'LEuUE#:R --insecure

All we need to do is save both the credentials in a file

tv:(()KW5$`>@/c:dB6_=.4|{jv?@!
.B_ES205}Mjzrh}G{emJi'LEuUE#:R

and then

cat credentials.txt | mc admin user add alias/

The PR should be simplified and not add new confusing variables.

dormanze · 2022-07-07T05:52:10Z

#accessKey is : tv:(()KW5$>@/c:dB6_=.4|{jv?@! #secretKey is : .B_ES205}Mjzrh}G{emJi'LEuUE#:R mc admin user add myminio tv:(()KW5$>@/c:dB6_=.4|{jv?@! .B_ES205}Mjzrh}G{emJi'LEuUE#:R --insecure

All we need to do is save both the credentials in a file
tv:(()KW5$`>@/c:dB6_=.4|{jv?@!
.B_ES205}Mjzrh}G{emJi'LEuUE#:R
and then
cat credentials.txt | mc admin user add alias/
The PR should be simplified and not add new confusing variables.

This is really a good idea. I'll revise my pr.

Add two parameters to prevent shell execution failure caused by special characters

…_helm

dormanze · 2022-07-07T06:51:10Z

@harshavardhana Please review again.thank you very much.

Add comments

helm/minio/templates/_helper_create_user.txt

Modification suggestions.

minio-trusted · 2022-07-08T06:03:18Z

Mint Automation

Test	Result
mint-large-bucket.sh	✔️
mint-gateway-s3.sh	✔️
mint-erasure.sh	✔️
mint-dist-erasure.sh	✔️
mint-compress-encrypt-dist-erasure.sh	✔️
mint-pools.sh	✔️
mint-fs.sh	❌ more...

15243-5c1451f/mint-fs.sh.log:

Running with
SERVER_ENDPOINT:      minio-k8s7:32587
ACCESS_KEY:           minio
SECRET_KEY:           ***REDACTED***
ENABLE_HTTPS:         0
SERVER_REGION:        us-east-1
MINT_DATA_DIR:        /mint/data
MINT_MODE:            full
ENABLE_VIRTUAL_STYLE: 0
RUN_ON_FAIL:          0

To get logs, run 'docker cp :/mint/log /tmp/mint-logs'

(1/15) Running aws-sdk-go tests ... done in 8 seconds
(2/15) Running aws-sdk-java tests ... done in 1 seconds
(3/15) Running aws-sdk-php tests ... done in 42 seconds
(4/15) Running aws-sdk-ruby tests ... done in 5 seconds
(5/15) Running awscli tests ... done in 52 seconds
(6/15) Running healthcheck tests ... done in 0 seconds
(7/15) Running mc tests ... done in 22 seconds
FAILED in 5 minutes and 10 seconds
Unhandled exception. System.AggregateException: One or more errors occurred. (A task was canceled.)
 ---> System.Threading.Tasks.TaskCanceledException: A task was canceled.
   at System.Threading.Tasks.Task.GetExceptions(Boolean includeTaskCanceledExceptions)
   at System.Threading.Tasks.Task.ThrowIfExceptional(Boolean includeTaskCanceledExceptions)
   at System.Threading.Tasks.Task.Wait(Int32 millisecondsTimeout, CancellationToken cancellationToken)
   at System.Threading.Tasks.Task.Wait()
   at Minio.Functional.Tests.Program.Main(String[] args) in /mint/run/core/minio-dotnet/temp/Minio.Functional.Tests/Program.cs:line 156
--- End of stack trace from previous location ---

   --- End of inner exception stack trace ---
   at System.Threading.Tasks.Task.ThrowIfExceptional(Boolean includeTaskCanceledExceptions)
   at System.Threading.Tasks.Task.Wait(Int32 millisecondsTimeout, CancellationToken cancellationToken)
   at System.Threading.Tasks.Task.Wait()
   at Minio.Functional.Tests.Program.Main(String[] args) in /mint/run/core/minio-dotnet/temp/Minio.Functional.Tests/Program.cs:line 156
(8/15) Running minio-go tests ... done in 56 seconds
(9/15) Running minio-java tests ... done in 19 seconds
(10/15) Running minio-js tests ... done in 59 seconds
(11/15) Running minio-py tests ... done in 45 seconds
(12/15) Running s3cmd tests ... done in 42 seconds
(13/15) Running s3select tests ... done in 1 seconds
(14/15) Running versioning tests ... done in 3 minutes and 2 seconds

Executed 14 out of 15 tests successfully.

Deleting image on docker hub
Deleting image locally

sathieu · 2022-07-22T10:52:52Z

@dormanze The chart is now broken with:

  users:
  - accessKey: "foobar"
    existingSecret: minio-secretkeys-gitops
    existingSecretKey: "foobar"
    policy: "readwrite_test"

The previous behavior was to use accessKey as secret key.

sathieu · 2022-07-22T11:03:58Z

Proposed fix: #15386

dormanze · 2022-07-22T11:12:47Z

@dormanze The chart is now broken with:

  users:
  - accessKey: "foobar"
    existingSecret: minio-secretkeys-gitops
    existingSecretKey: "foobar"
    policy: "readwrite_test"

LGTM

dormanze and others added 4 commits July 5, 2022 09:57

Update service.yaml

92e6e16

Merge branch 'minio:master' into fix_helm

d322e2a

fix helm failed

5b7e3aa

Add two parameters to prevent shell execution failure caused by special characters

Merge branch 'minio:master' into fix_helm

d4a10d9

Merge branch 'fix_helm' of https://github.com/dormanze/minio into fix…

0e7aa6e

…_helm

Update _helper_create_user.txt

5bacd95

upgrade comments.

harshavardhana requested changes Jul 7, 2022

View reviewed changes

z30001483 added 2 commits July 7, 2022 14:47

fix helm failed

5fd730c

Add two parameters to prevent shell execution failure caused by special characters

Merge branch 'fix_helm' of https://github.com/dormanze/minio into fix…

fc4f4c8

…_helm

Update _helper_create_user.txt

7fa08aa

Add comments

harshavardhana reviewed Jul 8, 2022

View reviewed changes

helm/minio/templates/_helper_create_user.txt Outdated Show resolved Hide resolved

harshavardhana requested changes Jul 8, 2022

View reviewed changes

helm/minio/templates/_helper_create_user.txt Outdated Show resolved Hide resolved

helm/minio/templates/_helper_create_user.txt Outdated Show resolved Hide resolved

helm/minio/templates/_helper_create_user.txt Outdated Show resolved Hide resolved

Update _helper_create_user.txt

5c1451f

Modification suggestions.

dormanze requested a review from harshavardhana July 8, 2022 07:34

harshavardhana approved these changes Jul 8, 2022

View reviewed changes

harshavardhana merged commit ab9544c into minio:master Jul 8, 2022

dormanze deleted the fix_helm branch July 15, 2022 07:28

sathieu added a commit to sathieu/minio that referenced this pull request Jul 22, 2022

helm: fix regression of minio#15243

5f3fb0b

The previous behavior was to use accessKey as secret key.

sathieu mentioned this pull request Jul 22, 2022

helm: fix regression of #15243 #15386

Merged

7 tasks

dormanze restored the fix_helm branch July 31, 2022 02:50

dormanze deleted the fix_helm branch July 31, 2022 02:51

bh4t added the bugfix label Jun 8, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Helm chart does not support special characters in access/secret key #15243

Fix Helm chart does not support special characters in access/secret key #15243

dormanze commented Jul 7, 2022 •

edited

harshavardhana commented Jul 7, 2022

dormanze commented Jul 7, 2022

harshavardhana left a comment

dormanze commented Jul 7, 2022 •

edited

harshavardhana commented Jul 7, 2022

dormanze commented Jul 7, 2022

dormanze commented Jul 7, 2022

minio-trusted commented Jul 8, 2022

sathieu commented Jul 22, 2022

sathieu commented Jul 22, 2022

dormanze commented Jul 22, 2022

Fix Helm chart does not support special characters in access/secret key #15243

Fix Helm chart does not support special characters in access/secret key #15243

Conversation

dormanze commented Jul 7, 2022 • edited

Description

Motivation and Context

How to test this PR?

Types of changes

Checklist:

harshavardhana commented Jul 7, 2022

dormanze commented Jul 7, 2022

harshavardhana left a comment

Choose a reason for hiding this comment

dormanze commented Jul 7, 2022 • edited

harshavardhana commented Jul 7, 2022

dormanze commented Jul 7, 2022

dormanze commented Jul 7, 2022

minio-trusted commented Jul 8, 2022

Mint Automation

15243-5c1451f/mint-fs.sh.log:

sathieu commented Jul 22, 2022

sathieu commented Jul 22, 2022

dormanze commented Jul 22, 2022

dormanze commented Jul 7, 2022 •

edited

dormanze commented Jul 7, 2022 •

edited