Add azure-workload auth to MSSQL scaler #6161
Conversation
rickbrouwer
force-pushed
the
mssql
branch
8 times, most recently
from
877148b to
e253199
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
rickbrouwer
force-pushed
the
mssql
branch
19 times, most recently
from
633987e to
8cd724e
Compare
my sincere apologies |
|
No worries |
|
/run-e2e * |
|
I think that we could cover this with e2e test as we have an Azure subscription. Could you open a PR to the testing infra repo to spin up and configure the database? |
Is a feature request in https://github.com/kedacore/testing-infrastructure ok? |
yes 😄 |
|
The testing infrastructure has to be deployed but @rickbrouwer needs help with that and I've not had time yet |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. |
|
This issue has been automatically closed due to inactivity. |
|
This is a good remainder to add the missing envs, I'll try to apply the changes during the weekend |
|
The MSSQL server is already created and configured -> kedacore/testing-infrastructure#160 (comment) |
Signed-off-by: rickbrouwer <rickbrouwer@gmail.com>
stale
bot
removed
the
stale
Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>
Signed-off-by: rickbrouwer <rickbrouwer@gmail.com>
Signed-off-by: rickbrouwer <rickbrouwer@gmail.com>
Signed-off-by: rickbrouwer <rickbrouwer@gmail.com>
| @@ -4,7 +4,9 @@ | |||
| package mssql_test | |||
There was a problem hiding this comment.
I think that we should split this file into 2, the old one just testing base mssql and another one within the azure folder to cover the workload identity integration. This will improve the readability of the test and also will help to detect the root cause of the failures (we did the same with prometheus e2e for example)
|
/run-e2e mssql |
|
Great!! to see the PR progressing!! I see that the E2E test failed? Can some one share the issue here? What set of things are remaining for the PR to get merged? |
The e2e test still needs to be split and there are definitely still one or more errors in the design of the e2e test. Otherwise I will continue with this later, but that may take a while. |
| azureSQLServerFQDN = os.Getenv("TF_AZURE_SQL_SERVER_FQDN") | ||
| azureSQLDBName = os.Getenv("TF_AZURE_SQL_SERVER_DB_NAME") | ||
| azureADTenantID = os.Getenv("TF_AZURE_SP_TENANT") | ||
| azureClientID = os.Getenv("AZURE_CLIENT_ID") |
There was a problem hiding this comment.
The env var that contains the app id is this:
| azureClientID = os.Getenv("AZURE_CLIENT_ID") | |
| azureClientID = os.Getenv("TF_AZURE_IDENTITY_1_APP_ID") |
There was a problem hiding this comment.
I case of necessity (to spin up the database or so), the admin credentials are in these envs:
- TF_AZURE_SQL_SERVER_ADMIN_USERNAME
- TF_AZURE_SQL_SERVER_ADMIN_PASSWORD
Checklist
Fixes #6104
Relates to #5797