Skip to content

Commit

Permalink
Fixing tests
Browse files Browse the repository at this point in the history
  • Loading branch information
karpikpl committed Jun 11, 2024
1 parent f87da6c commit ac0c64b
Show file tree
Hide file tree
Showing 4 changed files with 124 additions and 48 deletions.
14 changes: 9 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,12 @@ The specification below describes the `trigger` metadata in `ScaledObject` resou
- type: external
metadata:
scalerAddress: external-scaler-azure-cosmos-db.keda:4050 # Mandatory. Address of the external scaler service.
connection: <connection> # Mandatory. Connection string of Cosmos DB account with monitored container.
connection: <connection> # Optional. Connection string of Cosmos DB account with monitored container. Either `connection` or `endpoint` has to be provided.
endpoint: <endpoint> # Optional. Cosmos DB endpoint with monitored container. Either `connection` or `endpoint` has to be provided.
databaseId: <database-id> # Mandatory. ID of Cosmos DB database containing monitored container.
containerId: <container-id> # Mandatory. ID of monitored container.
leaseConnection: <lease-connection> # Mandatory. Connection string of Cosmos DB account with lease container.
leaseConnection: <lease-connection> # Optional. Connection string of Cosmos DB account with lease container. Either `leaseConnection` or `leaseEndpoint` has to be provided.
leaseEndpoint: <lease-endpoint> # Optional. Cosmos DB endpoint with lease container. Either `leaseConnection` or `leaseEndpoint` has to be provided.
leaseDatabaseId: <lease-database-id> # Mandatory. ID of Cosmos DB database containing lease container.
leaseContainerId: <lease-container-id> # Mandatory. ID of lease container.
processorName: <processor-name> # Mandatory. Name of change-feed processor used by listener application.
Expand All @@ -81,18 +83,20 @@ The specification below describes the `trigger` metadata in `ScaledObject` resou

- **`scalerAddress`** - Address of the external scaler service. This would be in format `<scaler-name>.<scaler-namespace>:<port>`. If you installed Azure Cosmos DB external scaler Helm chart in `keda` namespace and did not specify custom values, the metadata value would be `external-scaler-azure-cosmos-db.keda:4050`.

- **`connection`** - Connection string of the Cosmos DB account that contains the monitored container.
- **`connection`** or **`endpoint`** - Connection string of the Cosmos DB account or Cosmos DB endpoint that contains the monitored container.

- **`databaseId`** - ID of Cosmos DB database that contains the monitored container.

- **`containerId`** - ID of the monitored container.

- **`leaseConnection`** - Connection string of the Cosmos DB account that contains the lease container. This can be same or different from the value of `connection` metadata.
- **`leaseConnection`** or **`leaseEndpoint`**- Connection string of the Cosmos DB account or Cosmos DB endpoint that contains the lease container. This can be same or different from the value of `connection` metadata.

- **`leaseDatabaseId`** - ID of Cosmos DB database that contains the lease container. This can be same or different from the value of `databaseId` metadata.

- **`leaseContainerId`** - ID of the lease container containing the change feeds.

- **`processorName`** - Name of change-feed processor used by listener application. For more information on this, you can refer to [Implementing the change feed processor](https://docs.microsoft.com/azure/cosmos-db/sql/change-feed-processor#implementing-the-change-feed-processor) section.

> **Note** Ideally, we would have created `TriggerAuthentication` resource that would have prevented us from adding the connection strings in plain text in the `ScaledObject` trigger metadata. However, this is not possible since at the moment, the triggers of `external` type do not support referencing a `TriggerAuthentication` resource ([link](https://keda.sh/docs/scalers/external/#authentication-parameters)).
### Workload Identity support

To utilize Azure Workload Identity via Default Azure Credential use **`endpoint`** and **`leaseEndpoint`** parameters.
130 changes: 89 additions & 41 deletions src/Scaler.Tests/CosmosDbScalerServiceTests.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using System.Threading.Tasks;
using Google.Protobuf.Collections;
using Moq;
Expand All @@ -18,10 +19,8 @@ public CosmosDbScalerServiceTests()
}

[Theory]
[InlineData("connection")]
[InlineData("databaseId")]
[InlineData("containerId")]
[InlineData("leaseConnection")]
[InlineData("leaseDatabaseId")]
[InlineData("leaseContainerId")]
[InlineData("processorName")]
Expand All @@ -31,29 +30,31 @@ await Assert.ThrowsAsync<JsonSerializationException>(
() => _cosmosDbScalerService.IsActive(GetScaledObjectRefWithoutMetadata(metadataKey), null));
}

[Fact]
public async Task IsActive_ReturnsFalseOnZeroPartitions()
[Theory]
[InlineData(true)]
[InlineData(false)]
public async Task IsActive_ReturnsFalseOnZeroPartitions(bool workloadIdentity)
{
_metricProviderMock.Setup(provider => provider.GetPartitionCountAsync(It.IsAny<ScalerMetadata>())).ReturnsAsync(0L);
IsActiveResponse response = await _cosmosDbScalerService.IsActive(GetScaledObjectRef(), null);
IsActiveResponse response = await _cosmosDbScalerService.IsActive(GetScaledObjectRef(workloadIdentity), null);
Assert.False(response.Result);
}

[Theory]
[InlineData(1L)]
[InlineData(100L)]
public async Task IsActive_ReturnsFalseOnNonZeroPartitions(long partitionCount)
[InlineData(1L, true)]
[InlineData(1L, false)]
[InlineData(100L, true)]
[InlineData(100L, false)]
public async Task IsActive_ReturnsFalseOnNonZeroPartitions(long partitionCount, bool workloadIdentity)
{
_metricProviderMock.Setup(provider => provider.GetPartitionCountAsync(It.IsAny<ScalerMetadata>())).ReturnsAsync(partitionCount);
IsActiveResponse response = await _cosmosDbScalerService.IsActive(GetScaledObjectRef(), null);
IsActiveResponse response = await _cosmosDbScalerService.IsActive(GetScaledObjectRef(workloadIdentity), null);
Assert.True(response.Result);
}

[Theory]
[InlineData("connection")]
[InlineData("databaseId")]
[InlineData("containerId")]
[InlineData("leaseConnection")]
[InlineData("leaseDatabaseId")]
[InlineData("leaseContainerId")]
[InlineData("processorName")]
Expand All @@ -64,13 +65,16 @@ await Assert.ThrowsAsync<JsonSerializationException>(
}

[Theory]
[InlineData(0L)]
[InlineData(1L)]
[InlineData(100L)]
public async Task GetMetrics_ReturnsPartitionCount(long partitionCount)
[InlineData(0L, true)]
[InlineData(0L, false)]
[InlineData(1L, true)]
[InlineData(1L, false)]
[InlineData(100L, true)]
[InlineData(100L, false)]
public async Task GetMetrics_ReturnsPartitionCount(long partitionCount, bool workloadIdentity)
{
_metricProviderMock.Setup(provider => provider.GetPartitionCountAsync(It.IsAny<ScalerMetadata>())).ReturnsAsync(partitionCount);
GetMetricsResponse response = await _cosmosDbScalerService.GetMetrics(GetGetMetricsRequest(), null);
GetMetricsResponse response = await _cosmosDbScalerService.GetMetrics(GetGetMetricsRequest(workloadIdentity), null);

Assert.Single(response.MetricValues);

Expand All @@ -82,14 +86,16 @@ public async Task GetMetrics_ReturnsPartitionCount(long partitionCount)
}

[Theory]
[InlineData("")]
[InlineData("custom-metric-name")]
public async Task GetMetrics_ReturnsSameMetricNameIfPassed(string requestMetricName)
[InlineData("", true)]
[InlineData("", false)]
[InlineData("custom-metric-name", true)]
[InlineData("custom-metric-name", false)]
public async Task GetMetrics_ReturnsSameMetricNameIfPassed(string requestMetricName, bool workloadIdentity)
{
_metricProviderMock.Setup(provider => provider.GetPartitionCountAsync(It.IsAny<ScalerMetadata>())).ReturnsAsync(1L);

// No assertion with request.MetricName since it is ignored.
GetMetricsRequest request = GetGetMetricsRequest();
GetMetricsRequest request = GetGetMetricsRequest(workloadIdentity);
request.ScaledObjectRef.ScalerMetadata["metricName"] = requestMetricName;

GetMetricsResponse response = await _cosmosDbScalerService.GetMetrics(request, null);
Expand All @@ -99,10 +105,8 @@ public async Task GetMetrics_ReturnsSameMetricNameIfPassed(string requestMetricN
}

[Theory]
[InlineData("connection")]
[InlineData("databaseId")]
[InlineData("containerId")]
[InlineData("leaseConnection")]
[InlineData("leaseDatabaseId")]
[InlineData("leaseContainerId")]
[InlineData("processorName")]
Expand All @@ -112,10 +116,30 @@ await Assert.ThrowsAsync<JsonSerializationException>(
() => _cosmosDbScalerService.GetMetricSpec(GetScaledObjectRefWithoutMetadata(metadataKey), null));
}

[Fact]
public async Task GetMetricSpec_ReturnsMetricSpec()
[Theory]
[InlineData(true)]
[InlineData(false)]
public async Task GetMetricSpec_DoesNotThrowsOnOptionalMetadata(bool workloadIdentity)
{
await _cosmosDbScalerService.GetMetricSpec(GetScaledObjectRef(workloadIdentity), null);
}

[Theory]
[InlineData("endpoint", "connection")]
[InlineData("leaseEndpoint", "leaseConnection")]
public async Task GetMetricSpec_ThrowsOnMissingConnections(string firstMetadataKey, string secondMetadataKey)
{
var exception = await Assert.ThrowsAnyAsync<Exception>(
() => _cosmosDbScalerService.GetMetricSpec(GetScaledObjectRefWithoutMetadata(firstMetadataKey, secondMetadataKey), null));
Assert.IsType<JsonSerializationException>(exception.InnerException);
}

[Theory]
[InlineData(true)]
[InlineData(false)]
public async Task GetMetricSpec_ReturnsMetricSpec(bool workloadIdentity)
{
GetMetricSpecResponse response = await _cosmosDbScalerService.GetMetricSpec(GetScaledObjectRef(), null);
GetMetricSpecResponse response = await _cosmosDbScalerService.GetMetricSpec(GetScaledObjectRef(workloadIdentity), null);

Assert.Single(response.MetricSpecs);

Expand All @@ -127,11 +151,13 @@ public async Task GetMetricSpec_ReturnsMetricSpec()
}

[Theory]
[InlineData("")]
[InlineData("custom-metric-name")]
public async Task GetMetricSpec_ReturnsSameMetricNameIfPassed(string requestMetricName)
[InlineData("", true)]
[InlineData("", false)]
[InlineData("custom-metric-name", true)]
[InlineData("custom-metric-name", false)]
public async Task GetMetricSpec_ReturnsSameMetricNameIfPassed(string requestMetricName, bool workloadIdentity)
{
ScaledObjectRef request = GetScaledObjectRef();
ScaledObjectRef request = GetScaledObjectRef(workloadIdentity);
request.ScalerMetadata["metricName"] = requestMetricName;

GetMetricSpecResponse response = await _cosmosDbScalerService.GetMetricSpec(request, null);
Expand All @@ -140,11 +166,20 @@ public async Task GetMetricSpec_ReturnsSameMetricNameIfPassed(string requestMetr
Assert.Equal(requestMetricName, response.MetricSpecs[0].MetricName);
}

[Fact]
public async Task GetMetricSpec_ReturnsNormalizedMetricName()
[Theory]
[InlineData(true)]
[InlineData(false)]
public async Task GetMetricSpec_ReturnsNormalizedMetricName(bool workloadIdentity)
{
ScaledObjectRef request = GetScaledObjectRef();
request.ScalerMetadata["leaseConnection"] = "AccountEndpoint=https://example.com:443/;AccountKey=ZHVtbXky";
ScaledObjectRef request = GetScaledObjectRef(workloadIdentity);
if (workloadIdentity)
{
request.ScalerMetadata["leaseEndpoint"] = "https://example.com:443";
}
else
{
request.ScalerMetadata["leaseConnection"] = "AccountEndpoint=https://example.com:443/;AccountKey=ZHVtbXky";
}
request.ScalerMetadata["leaseDatabaseId"] = "Dummy.Lease.Database.Id";
request.ScalerMetadata["leaseContainerId"] = "Dummy:Lease:Container:Id";
request.ScalerMetadata["processorName"] = "Dummy%Processor%Name";
Expand All @@ -158,12 +193,12 @@ public async Task GetMetricSpec_ReturnsNormalizedMetricName()
response.MetricSpecs[0].MetricName);
}

private static GetMetricsRequest GetGetMetricsRequest()
private static GetMetricsRequest GetGetMetricsRequest(bool workloadIdentity)
{
return new GetMetricsRequest
{
MetricName = "dummy-metric-name",
ScaledObjectRef = GetScaledObjectRef(),
ScaledObjectRef = GetScaledObjectRef(workloadIdentity),
};
}

Expand All @@ -176,15 +211,20 @@ private static GetMetricsRequest GetGetMetricsRequestWithoutMetadata(string meta
};
}

private static ScaledObjectRef GetScaledObjectRefWithoutMetadata(string metadataKey)
private static ScaledObjectRef GetScaledObjectRefWithoutMetadata(params string[] metadataKeys)
{
var scaledObjectRef = GetScaledObjectRef();
scaledObjectRef.ScalerMetadata.Remove(metadataKey);
var scaledObjectRef = GetScaledObjectRef(workloadIdentity: true);
// this is not technically correct but for sake of the test we need both connection and endpoint to be present
scaledObjectRef.ScalerMetadata["endpoint"] = "https://example1.com:443";
scaledObjectRef.ScalerMetadata["leaseEndpoint"] = "https://example2.com:443";

foreach (string metadataKey in metadataKeys)
scaledObjectRef.ScalerMetadata.Remove(metadataKey);

return scaledObjectRef;
}

private static ScaledObjectRef GetScaledObjectRef()
private static ScaledObjectRef GetScaledObjectRef(bool workloadIdentity = false)
{
var scaledObjectRef = new ScaledObjectRef
{
Expand All @@ -194,10 +234,18 @@ private static ScaledObjectRef GetScaledObjectRef()

MapField<string, string> scalerMetadata = scaledObjectRef.ScalerMetadata;

scalerMetadata["connection"] = "AccountEndpoint=https://example1.com:443/;AccountKey=ZHVtbXkx";
if (workloadIdentity)
{
scalerMetadata["endpoint"] = "https://example1.com:443";
scalerMetadata["leaseEndpoint"] = "https://example2.com:443";
}
else
{
scalerMetadata["connection"] = "AccountEndpoint=https://example1.com:443/;AccountKey=ZHVtbXkx";
scalerMetadata["leaseConnection"] = "AccountEndpoint=https://example2.com:443/;AccountKey=ZHVtbXky";
}
scalerMetadata["databaseId"] = "dummy-database-id";
scalerMetadata["containerId"] = "dummy-container-id";
scalerMetadata["leaseConnection"] = "AccountEndpoint=https://example2.com:443/;AccountKey=ZHVtbXky";
scalerMetadata["leaseDatabaseId"] = "dummy-lease-database-id";
scalerMetadata["leaseContainerId"] = "dummy-lease-container-id";
scalerMetadata["processorName"] = "dummy-processor-name";
Expand Down
4 changes: 2 additions & 2 deletions src/Scaler/Services/CosmosDbMetricProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ public async Task<long> GetPartitionCountAsync(ScalerMetadata scalerMetadata)
try
{
Container leaseContainer = _factory
.GetCosmosClient(scalerMetadata.LeaseConnection)
.GetCosmosClient(scalerMetadata.LeaseConnection ?? scalerMetadata.LeaseEndpoint)
.GetContainer(scalerMetadata.LeaseDatabaseId, scalerMetadata.LeaseContainerId);

ChangeFeedEstimator estimator = _factory
.GetCosmosClient(scalerMetadata.Connection)
.GetCosmosClient(scalerMetadata.Connection ?? scalerMetadata.Endpoint)
.GetContainer(scalerMetadata.DatabaseId, scalerMetadata.ContainerId)
.GetChangeFeedEstimator(scalerMetadata.ProcessorName, leaseContainer);

Expand Down
24 changes: 24 additions & 0 deletions src/Scaler/Services/ScalerMetadata.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Data.Common;
using System.Runtime.Serialization;
using Newtonsoft.Json;

namespace Keda.CosmosDb.Scaler
Expand All @@ -9,10 +10,16 @@ internal sealed class ScalerMetadata
{
private string _metricName;

[JsonProperty(Required = Required.Default)]
public string Connection { get; set; }
[JsonProperty(Required = Required.Default)]
public string Endpoint { get; set; }
public string DatabaseId { get; set; }
public string ContainerId { get; set; }
[JsonProperty(Required = Required.Default)]
public string LeaseConnection { get; set; }
[JsonProperty(Required = Required.Default)]
public string LeaseEndpoint { get; set; }
public string LeaseDatabaseId { get; set; }
public string LeaseContainerId { get; set; }
public string ProcessorName { get; set; }
Expand All @@ -39,6 +46,10 @@ private string LeaseAccountHost
{
get
{
if (string.IsNullOrEmpty(LeaseConnection))
{
return new Uri(LeaseEndpoint).Host;
}
var builder = new DbConnectionStringBuilder { ConnectionString = this.LeaseConnection };
return new Uri((string)builder["AccountEndpoint"]).Host;
}
Expand All @@ -48,5 +59,18 @@ public static ScalerMetadata Create(ScaledObjectRef scaledObjectRef)
{
return JsonConvert.DeserializeObject<ScalerMetadata>(scaledObjectRef.ScalerMetadata.ToString());
}

[OnDeserialized]
internal void OnDeserializedMethod(StreamingContext context)
{
if (string.IsNullOrEmpty(LeaseConnection) && string.IsNullOrEmpty(LeaseEndpoint))
{
throw new JsonSerializationException("Both LeaseConnection and LeaseEndpoint are missing.");
}
if(string.IsNullOrEmpty(Connection) && string.IsNullOrEmpty(Endpoint))
{
throw new JsonSerializationException("Both Connection and Endpoint are missing.");
}
}
}
}

0 comments on commit ac0c64b

Please sign in to comment.