Skip to content

Commit

Permalink
fixes
Browse files Browse the repository at this point in the history
Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>
  • Loading branch information
rickbrouwer committed Sep 14, 2024
1 parent fa68ac5 commit 5d325f8
Show file tree
Hide file tree
Showing 3 changed files with 128 additions and 48 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ Here is an overview of all new **experimental** features:
- **GCP Scalers**: Added custom time horizon in GCP scalers ([#5778](https://github.com/kedacore/keda/issues/5778))
- **GitHub Scaler**: Fixed pagination, fetching repository list ([#5738](https://github.com/kedacore/keda/issues/5738))
- **Kafka**: Fix logic to scale to zero on invalid offset even with earliest offsetResetPolicy ([#5689](https://github.com/kedacore/keda/issues/5689))
- **MSSQL Scaler**: Add azure-workload auth ([#6104](https://github.com/kedacore/keda/issues/6104))
- **RabbitMQ Scaler**: Add connection name for AMQP ([#5958](https://github.com/kedacore/keda/issues/5958))
- TODO ([#XXX](https://github.com/kedacore/keda/issues/XXX))

Expand Down
10 changes: 4 additions & 6 deletions pkg/scalers/mssql_scaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,9 @@ package scalers
import (
"context"
"database/sql"
"errors"
"fmt"
"net"
"net/url"
"strconv"

// mssql driver required for this scaler
_ "github.com/denisenkom/go-mssqldb"
Expand Down Expand Up @@ -42,13 +40,13 @@ type mssqlMetadata struct {
WorkloadIdentityTenantID string `keda:"name=WorkloadIdentityTenantID,order=authParams;triggerMetadata,optional"`
WorkloadIdentityAuthorityHost string `keda:"name=WorkloadIdentityAuthorityHost,order=authParams;triggerMetadata,optional"`
WorkloadIdentityResource string `keda:"name=WorkloadIdentityResource,order=authParams;triggerMetadata,optional"`
TriggerIndex int

TriggerIndex int
}

func (m *mssqlMetadata) Validate() error {
if m.TargetValue == 0 {
return errors.New("no targetValue given")
if m.ConnectionString == "" && m.Host == "" {
return fmt.Errorf("must provide either connectionstring or host")
}
return nil
}
Expand Down
165 changes: 123 additions & 42 deletions pkg/scalers/mssql_scaler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,33 +5,118 @@ import (
"testing"

"github.com/stretchr/testify/assert"

"github.com/kedacore/keda/v2/apis/keda/v1alpha1"
"github.com/kedacore/keda/v2/pkg/scalers/scalersconfig"
kedautil "github.com/kedacore/keda/v2/pkg/util"
)

type parseMSSQLMetadataTestData struct {
name string
metadata map[string]string
resolvedEnv map[string]string
authParams map[string]string
podIdentity v1alpha1.AuthPodIdentity
expectedError string
name string
metadata map[string]string
resolvedEnv map[string]string
authParams map[string]string
podIdentity v1alpha1.AuthPodIdentity
expectedError string
expectedConnectionString string
expectedMetricName string
}

var testMSSQLMetadata = []parseMSSQLMetadataTestData{
{
name: "Valid metadata",
metadata: map[string]string{"query": "SELECT 1", "targetValue": "1", "host": "localhost"},
resolvedEnv: map[string]string{},
authParams: map[string]string{},
expectedError: "",
name: "Direct connection string input",
metadata: map[string]string{"query": "SELECT 1", "targetValue": "1"},
resolvedEnv: map[string]string{},
authParams: map[string]string{"connectionString": "sqlserver://localhost"},
expectedConnectionString: "sqlserver://localhost",
},
{
name: "Direct connection string input with activationTargetValue",
metadata: map[string]string{"query": "SELECT 1", "targetValue": "1", "activationTargetValue": "20"},
resolvedEnv: map[string]string{},
authParams: map[string]string{"connectionString": "sqlserver://localhost"},
expectedConnectionString: "sqlserver://localhost",
},
{
name: "Direct connection string input, OLEDB format",
metadata: map[string]string{"query": "SELECT 1", "targetValue": "1"},
resolvedEnv: map[string]string{},
authParams: map[string]string{"connectionString": "Server=example.database.windows.net;port=1433;Database=AdventureWorks;Persist Security Info=False;User ID=user1;Password=Password#1;MultipleActiveResultSets=False;Encrypt=True;TrustServerCertificate=False;Connection Timeout=30;"},
expectedConnectionString: "Server=example.database.windows.net;port=1433;Database=AdventureWorks;Persist Security Info=False;User ID=user1;Password=Password#1;MultipleActiveResultSets=False;Encrypt=True;TrustServerCertificate=False;Connection Timeout=30;",
},
{
name: "Connection string input via environment variables",
metadata: map[string]string{"query": "SELECT 1", "targetValue": "1", "connectionStringFromEnv": "test_connection_string"},
resolvedEnv: map[string]string{"test_connection_string": "sqlserver://localhost?database=AdventureWorks"},
authParams: map[string]string{},
expectedConnectionString: "sqlserver://localhost?database=AdventureWorks",
},
{
name: "Connection string generated from minimal required metadata",
metadata: map[string]string{"query": "SELECT 1", "targetValue": "1", "host": "127.0.0.1"},
resolvedEnv: map[string]string{},
authParams: map[string]string{},
expectedMetricName: "mssql",
expectedConnectionString: "sqlserver://127.0.0.1",
},
{
name: "Valid metadata with Azure Workload Identity",
name: "Connection string generated from full metadata",
metadata: map[string]string{"query": "SELECT 1", "targetValue": "1", "host": "example.database.windows.net", "username": "user1", "passwordFromEnv": "test_password", "port": "1433", "database": "AdventureWorks"},
resolvedEnv: map[string]string{"test_password": "Password#1"},
authParams: map[string]string{},
expectedConnectionString: "sqlserver://user1:Password%231@example.database.windows.net:1433?database=AdventureWorks",
},
{
name: "Variation of previous: no port, password from authParams, metricName from database name",
metadata: map[string]string{"query": "SELECT 1", "targetValue": "1", "host": "example.database.windows.net", "username": "user2", "database": "AdventureWorks"},
resolvedEnv: map[string]string{},
authParams: map[string]string{"password": "Password#2"},
expectedMetricName: "mssql",
expectedConnectionString: "sqlserver://user2:Password%232@example.database.windows.net?database=AdventureWorks",
},
{
name: "Connection string generated from full authParams",
metadata: map[string]string{"query": "SELECT 1", "targetValue": "1"},
resolvedEnv: map[string]string{},
authParams: map[string]string{"password": "Password#2", "host": "example.database.windows.net", "username": "user2", "database": "AdventureWorks", "port": "1433"},
expectedMetricName: "mssql",
expectedConnectionString: "sqlserver://user2:Password%232@example.database.windows.net:1433?database=AdventureWorks",
},
{
name: "Variation of previous: no database name, metricName from host",
metadata: map[string]string{"query": "SELECT 1", "targetValue": "1", "host": "example.database.windows.net", "username": "user3"},
resolvedEnv: map[string]string{},
authParams: map[string]string{"password": "Password#3"},
expectedMetricName: "mssql",
expectedConnectionString: "sqlserver://user3:Password%233@example.database.windows.net",
},
{
name: "Error: missing query",
metadata: map[string]string{"targetValue": "1"},
resolvedEnv: map[string]string{},
authParams: map[string]string{"connectionString": "sqlserver://localhost"},
expectedError: "missing required parameter \"query\" in [triggerMetadata]",
},
{
name: "Error: missing targetValue",
metadata: map[string]string{"query": "SELECT 1"},
resolvedEnv: map[string]string{},
authParams: map[string]string{"connectionString": "sqlserver://localhost"},
expectedError: "missing required parameter \"targetValue\" in [triggerMetadata]",
},
{
name: "Error: missing host",
metadata: map[string]string{"query": "SELECT 1", "targetValue": "1"},
resolvedEnv: map[string]string{},
authParams: map[string]string{},
expectedError: "must provide either connectionstring or host",
},
{
name: "Valid metadata with Azure Workload Identity",
metadata: map[string]string{
"query": "SELECT COUNT(*) FROM table",
"targetValue": "5",
"host": "mssql-server",
"host": "mssql-server.database.windows.net",
"port": "1433",
"database": "test-db",
},
Expand All @@ -40,37 +125,31 @@ var testMSSQLMetadata = []parseMSSQLMetadataTestData{
"workloadIdentityResource": "https://database.windows.net/",
},
podIdentity: v1alpha1.AuthPodIdentity{
Provider: v1alpha1.PodIdentityProviderAzureWorkload,
Provider: v1alpha1.PodIdentityProviderAzureWorkload,
IdentityID: kedautil.StringPointer("client-id"),
IdentityTenantID: kedautil.StringPointer("tenant-id"),
IdentityAuthorityHost: kedautil.StringPointer("https://login.microsoftonline.com/"),
},
expectedError: "",
},
{
name: "Missing query",
metadata: map[string]string{"targetValue": "1", "host": "localhost"},
resolvedEnv: map[string]string{},
authParams: map[string]string{},
expectedError: "missing required parameter \"query\"",
},
{
name: "Missing targetValue",
metadata: map[string]string{"query": "SELECT 1", "host": "localhost"},
resolvedEnv: map[string]string{},
authParams: map[string]string{},
expectedError: "missing required parameter \"targetValue\"",
},
{
name: "Invalid targetValue",
metadata: map[string]string{"query": "SELECT 1", "targetValue": "invalid", "host": "localhost"},
resolvedEnv: map[string]string{},
authParams: map[string]string{},
expectedError: "error parsing targetValue",
},
{
name: "Missing host",
metadata: map[string]string{"query": "SELECT 1", "targetValue": "1"},
name: "Azure Workload Identity without workloadIdentityResource",
metadata: map[string]string{
"query": "SELECT COUNT(*) FROM table",
"targetValue": "5",
"host": "mssql-server.database.windows.net",
"port": "1433",
"database": "test-db",
},
resolvedEnv: map[string]string{},
authParams: map[string]string{},
expectedError: "missing required parameter \"host\"",
authParams: map[string]string{},
podIdentity: v1alpha1.AuthPodIdentity{
Provider: v1alpha1.PodIdentityProviderAzureWorkload,
IdentityID: kedautil.StringPointer("client-id"),
IdentityTenantID: kedautil.StringPointer("tenant-id"),
IdentityAuthorityHost: kedautil.StringPointer("https://login.microsoftonline.com/"),
},
expectedError: "",
},
}

Expand All @@ -94,12 +173,14 @@ func TestParseMSSQLMetadata(t *testing.T) {

if testData.podIdentity.Provider == v1alpha1.PodIdentityProviderAzureWorkload {
assert.Equal(t, testData.authParams["workloadIdentityResource"], meta.WorkloadIdentityResource)
assert.NotEmpty(t, meta.WorkloadIdentityClientID)
assert.NotEmpty(t, meta.WorkloadIdentityTenantID)
assert.Equal(t, *testData.podIdentity.IdentityID, meta.WorkloadIdentityClientID)
assert.Equal(t, *testData.podIdentity.IdentityTenantID, meta.WorkloadIdentityTenantID)
assert.Equal(t, *testData.podIdentity.IdentityAuthorityHost, meta.WorkloadIdentityAuthorityHost)
} else {
assert.Empty(t, meta.WorkloadIdentityResource)
assert.Empty(t, meta.WorkloadIdentityClientID)
assert.Empty(t, meta.WorkloadIdentityTenantID)
assert.Empty(t, meta.WorkloadIdentityAuthorityHost)
}
}
})
Expand All @@ -123,7 +204,7 @@ func TestMSSQLGetMetricSpecForScaling(t *testing.T) {
assert.NoError(t, err)

mockMSSQLScaler := mssqlScaler{
metadata: *meta,
metadata: meta,
}

metricSpec := mockMSSQLScaler.GetMetricSpecForScaling(context.Background())
Expand Down

0 comments on commit 5d325f8

Please sign in to comment.