Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to set DAPR protocol for Container Apps #1100

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions RELEASE_NOTES.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
Release Notes
=============

## 1.8.10
* Container Apps: Add support for setting Dapr protocol (#1083).

## 1.8.9
* Managed Identity: Support for federated identity credentials.

Expand Down
1 change: 1 addition & 0 deletions docs/content/api-overview/resources/container-apps.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ The Container Apps builder (`containerApp`) is used to define one or more contai
| system_identity | Activates the system identity of the Azure Container App. |
| dapr_app_id | Sets the dapr app id for the app. |
| dapr_app_port | Sets the dapr app port for the app. |
| dapr_app_protocol | Sets the dapr app protocol for the app. |
| replicas | Sets the minimum and maximum replicas to scale the container app. |
| active_revision_mode | Indicates whether multiple version of a container app can be active at once.|
| add_registry_credentials | Adds container image registry credentials for images in this container app, which are a list of server and usernames. Passwords are supplied as secure parameters. |
Expand Down
5 changes: 4 additions & 1 deletion src/Farmer/Arm/App.fs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,9 @@ type ContainerApp =
ScaleRules: Map<string, ScaleRule>
Identity: ManagedIdentity
Replicas: {| Min: int; Max: int |} option
DaprConfig: {| AppId: string; Port: uint16 option |} option
DaprConfig: {| AppId: string
Port: uint16 option
Protocol: string option |} option
Secrets: Map<ContainerAppSettingKey, SecretValue>
EnvironmentVariables: Map<string, EnvVar>
ImageRegistryCredentials: ImageRegistryAuthentication list
Expand Down Expand Up @@ -313,6 +315,7 @@ type ContainerApp =
enabled = true
appId = settings.AppId
appPort = settings.Port |> Option.toNullable
appProtocol = settings.Protocol
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With Protocol now a union rather than a string, this code will need a pattern match to emit the correct string e.g.

match settings.Protocol with Http -> "http" | Gprc -> "grpc"

etc.

|}
:> obj
| None -> {| enabled = false |}
Expand Down
40 changes: 36 additions & 4 deletions src/Farmer/Builders/Builders.ContainerApps.fs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ type ContainerAppConfig =
Identity: ManagedIdentity
Replicas: {| Min: int; Max: int |} option
DaprConfig: {| AppId: string option
Port: uint16 option |} option
Port: uint16 option
Protocol: string option |} option
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would make a type for the protocol e.g.

[<RequireQualifiedAccess>]
type DaprProtocol = Grpc | Http

Doing this will make it easier for individuals to know what the available valid options are, rather than a raw string.

Secrets: Map<ContainerAppSettingKey, SecretValue>
EnvironmentVariables: Map<string, EnvVar>
Volumes: Map<string, Volume>
Expand Down Expand Up @@ -127,7 +128,12 @@ type ContainerEnvironmentConfig =
containerApp.DaprConfig
|> Option.map (fun x ->
match x.AppId with
| Some appId -> {| AppId = appId; Port = x.Port |}
| Some appId ->
{|
AppId = appId
Port = x.Port
Protocol = x.Protocol
|}
| None ->
raiseFarmer
$"The container app '{containerApp.Name.Value}' requires a Dapr App ID when Dapr is enabled.")
Expand Down Expand Up @@ -448,7 +454,12 @@ type ContainerAppBuilder() =
DaprConfig =
state.DaprConfig
|> Option.map (fun x -> {| x with AppId = Some appId |})
|> Option.defaultWith (fun () -> {| AppId = Some appId; Port = None |})
|> Option.defaultWith (fun () ->
{|
AppId = Some appId
Port = None
Protocol = None
|})
|> Some
}

Expand All @@ -459,7 +470,28 @@ type ContainerAppBuilder() =
DaprConfig =
state.DaprConfig
|> Option.map (fun x -> {| x with Port = Some port |})
|> Option.defaultWith (fun () -> {| AppId = None; Port = Some port |})
|> Option.defaultWith (fun () ->
{|
AppId = None
Port = Some port
Protocol = None
|})
|> Some
}

/// Configures Dapr protocol in the Azure Container App.
[<CustomOperation "dapr_app_protocol">]
member _.SetDaprProtocol(state: ContainerAppConfig, protocol) =
{ state with
DaprConfig =
state.DaprConfig
|> Option.map (fun x -> {| x with Protocol = Some protocol |})
|> Option.defaultWith (fun () ->
{|
AppId = None
Port = None
Protocol = Some protocol
|})
|> Some
}

Expand Down
6 changes: 6 additions & 0 deletions src/Tests/ContainerApps.fs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ let fullContainerAppDeployment =
ingress_target_port 80us
ingress_transport Auto
dapr_app_id "http"
dapr_app_port 5000us
dapr_app_protocol "grpc"
add_http_scale_rule "http-rule" { ConcurrentRequests = 100 }
}

Expand Down Expand Up @@ -414,6 +416,10 @@ let tests =
Expect.isNotNull ruleAuth "auth[0] was null"
Expect.equal (ruleAuth["secretRef"] |> string) connectionSecretName "Incorrect secretRef"
Expect.equal (ruleAuth["triggerParameter"] |> string) "connection" "Incorrect triggerParameter"

let daprConfig = httpContainerApp.SelectToken("properties.configuration.dapr")
Expect.equal (daprConfig["appPort"] |> uint16) 5000us "Incorrect dapr appPort"
Expect.equal (daprConfig["appProtocol"] |> string) "grpc" "Incorrect dapr appProtocol"
}

test "Makes container app with MSI" {
Expand Down