CBG-3791: change getAuthScopeHandleCreateDB method to not expand environment variables #6703
Conversation
…ronment variables
rest/admin_api.go
Outdated
| @@ -181,7 +181,14 @@ func getAuthScopeHandleCreateDB(ctx context.Context, h *handler) (bucketName str | |||
| Bucket string `json:"bucket"` | |||
| } | |||
| reader := bytes.NewReader(bodyJSON) | |||
| err = DecodeAndSanitiseConfig(ctx, reader, &dbConfigBody, false) | |||
|
|
|||
| // read and decode json to pull bucket name from the config | |||
There was a problem hiding this comment.
I think you can call readSantizedJson here.
The thing that might happen is that you need to call ConvertBackQuotedStrings to make it valid json. The test should include a config with backquotes
rest/adminapitest/admin_api_test.go
Outdated
| }, | ||
| "num_index_replicas": 0, | ||
| "enable_shared_bucket_access": true, | ||
| "use_views": false}`, |
There was a problem hiding this comment.
this might fail if SG_TEST_USE_XATTRS=false is set, can you check?
There was a problem hiding this comment.
Just checked and no failure.
rest/admin_api.go
Outdated
| reader := bytes.NewReader(bodyJSON) | ||
| err = DecodeAndSanitiseConfig(ctx, reader, &dbConfigBody, false) | ||
|
|
||
| // read and decode json to pull bucket name from the config | ||
| b, err := io.ReadAll(reader) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| // Expand environment variables if needed | ||
| if base.BoolDefault(h.server.Config.Unsupported.AllowDbConfigEnvVars, true) { | ||
| b, err = expandEnv(h.ctx(), b) |
There was a problem hiding this comment.
We're doing bytes->reader->bytes for no reason here with NewReader->ReadAll
We should be able to pass bodyJSON straight into expandEnv and ConvertBackQuotedStrings, since they don't modify the slice, they return a modified copy of it.
It does feels like we need to push those two pieces (expandEnv/ConvertBackQuotedStrings) into a shared function called by this, DecodeAndSanitiseConfig, and readSanitizeJSON.
We can pass a bool in to that function control env var behaviour and just have it take bytes and return bytes.
| // - Create CBS user to authenticate with over admin port to force auth scope callback call | ||
| // - Create db with sync function that calls env variable | ||
| // - Assert that db is created | ||
| func TestDatabaseCreationWithEnvVariable(t *testing.T) { |
There was a problem hiding this comment.
This test doesn't work for SG_TEST_USE_GSI=false or SG_TEST_USE_DEFAULT_COLLECTION=true
Will need to build the db config to accommodate those modes.
Some existing functions to consider using or adapting:
dbConfigForTestBucketmakeDbConfig
Should be able to include string values like "${VAR}" for sync function before marshalling it to pass into the request.
There was a problem hiding this comment.
Oh wow didn't even think about that, on it fixing it
CBG-3791
Pre-review checklist
fmt.Print,log.Print, ...)base.UD(docID),base.MD(dbName))docs/apiIntegration Tests
GSI=true,xattrs=truehttps://jenkins.sgwdev.com/job/SyncGateway-Integration/2327/