search

Azure / data-api-builder

Data API builder provides modern REST and GraphQL endpoints to your Azure Databases and on-prem stores.
https://aka.ms/dab/docs
MIT License
786 stars 142 forks source link

Ensure environment variables works for child files. #2201

Closed rohkhann closed 1 month ago

rohkhann commented 1 month ago

Why make this change?

When we are loading child config files, we were not setting replace env variable to true and hence child config environment variables were not being obeyed. This meant that when users were using @env('variable') instead of searching for the environment variable, the code took @env('vairable') as the literal string. This fix ensures that environment variables can be used for multi-db scenario. You need to add in parent config a data-source-files referencing a child config with connection string referencing a env variable:

  1. Parent config: "data-source": { "database-type": "mssql", "connection-string": "@env('my-connection-string')", "options": { "set-session-context": true } }, "data-source-files": [ "dab-config-dab.json" ]
  2. Child config named dab-config-dab.json: "data-source": { "database-type": "mssql", "connection-string": "@env('my-connection-string-dab')", "options": { "set-session-context": true } },

    What is this change?

    Adding true to runtimeconfig file.

How was this tested?

Ran an integration test with two config files and an environment variable in the child file.

rohkhann commented 1 month ago

/azp run

seantleonard commented 1 month ago

Ran an integration test with two config files and an environment variable in the child file.

can this be added as integration test?

rohkhann commented 1 month ago

Ran an integration test with two config files and an environment variable in the child file.

can this be added as integration test?

Maybe bit tricky as we dont have any test structure in the pipelines right now that can take 2 config files. Given dab's ga deadline and we want this fix to go in, maybe better to invest in that testing all up for multi-db post GA.

rohkhann commented 1 month ago

/azp run

seantleonard commented 1 month ago

PR description needs more details.

  1. Can you add a minimum reproducible example parent dab-config.json and child dab-child-config.json? (perhaps these files are already in the test project, perhaps just sample dab start command?
  2. Since there isn't an open issue tracking this, can you also note what error messages appeared when env_vars were not processed in the child config files?

Need to open issue to track adding a test: here are some notes for whoever picks it up:

  1. Test for this use case. This should be straightforward in the ConfigurationTests.cs class:
    • Set test-specific environment variable child-conf-connstring and put a value
    • create parent runtime config with data=sources property and child runtime config and write to disk (there are examples of writing custom config to disk in ConfigurationTests.cs
    • Start dab which implies that this constructor is called in RuntimeConfig.cs: public RuntimeConfig(string? Schema, DataSource DataSource, RuntimeEntities Entities, RuntimeOptions? Runtime = null, DataSourceFiles? DataSourceFiles = null)
    • Validate that whatever error message was encountered previous, is no longer experienced when not processing env vars in the child file.
seantleonard commented 1 month ago

/azp run