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

Fix: [Functions][Kubernetes]SyncTrigger issue #208

Conversation

TsuyoshiUshio
Copy link
Collaborator

@TsuyoshiUshio TsuyoshiUshio commented May 18, 2021

I open this PR for sharing / discussing the design of the fix.
@pragnagopa @divyagandhisethi @ahmelsayed @cgillum

On the Kudu Lite Side, We found these issues.

SSL Connection issue when we call SyncTrigger API from Azure Functions Host

That is caused when the API access to the Kubernetes API, however, the HTTP Client
doesn't refer to the /var/run/secrets/kubernetes.io/serviceaccount/ca.crt.

We have three ways to fix.

  1. Add HttpClientHandler for accepting Self-Signed-Certificate
  2. Add HttpClientHandler for implementing a custom ca.cert validation.
  3. Include the ca.cert when BuildServer startup cp /var/run/secrets/kubernetes.io/serviceaccount/ca.crt /usr/local/share/ca-certificates/ && update-ca-certificates.

Since the ca.crt is mounted and it is possible that is updated, so that the second solution is the best, however, IMO, we can fix the first one for a while and create a new issue for 2 might be the best.

Wrong URL name

The correct secret name is functionApp-secrets. There are the following secrets are found. Probably, functionApp-encryption-secrets is old one. functionApp-secrets contains more information.

$ kubectl get secrets
tsushiququecon1901                                     Opaque                                2      4d4h
tsushiququecon1901-encryptionkey-secrets               Opaque                                1      4d4h
tsushiququecon1901-keys                                Opaque                                2      4d4h
tsushiququecon1901-secrets                             Opaque                                12     4d4h
tsushiququecon1901-token-84s6f                         kubernetes.io/service-account-token   3      4d4h

token validation fails

The token validation fails, because of the Decryption using the different logic with Azure Functions Host. I change it to use the same logic as Azure Functions Host.

Sync Trigger Payload is not what is expected.

Current Code, It assume that the payload will be an array of the following.

            string jsonText = @"
            {
                ""functionName"": ""f1"",
                ""bindings"": [{
                    ""type"": ""eventGridTrigger"",
                    ""methods"": [""GET""],
                    ""authLevel"": ""anonymous""
                }]
            }";

However, the sync trigger payload that is coming from Azure Functions Host is

{
    "triggers": [
        {
            "name": "myQueueItem",
            "type": "queueTrigger",
            "direction": "in",
            "queueName": "js-queue-items",
            "connection": "AzureWebJobsStorage",
            "functionName": "QueueTrigger"
        }
    ],
    "functions": [
        {
            "name": "QueueTrigger",
            "script_root_path_href": "https://tsushiququecon1901.limaarcncsenv19--k53aemd.northcentralusstage.k4apps.io/admin/vfs/home/site/wwwroot/QueueTrigger/",
            "script_href": "https://tsushiququecon1901.limaarcncsenv19--k53aemd.northcentralusstage.k4apps.io/admin/vfs/home/site/wwwroot/QueueTrigger/index.js",
            "config_href": "https://tsushiququecon1901.limaarcncsenv19--k53aemd.northcentralusstage.k4apps.io/admin/vfs/home/site/wwwroot/QueueTrigger/function.json",
            "test_data_href": "https://tsushiququecon1901.limaarcncsenv19--k53aemd.northcentralusstage.k4apps.io/admin/vfs/tmp/FunctionsData/QueueTrigger.dat",
            "href": "https://tsushiququecon1901.limaarcncsenv19--k53aemd.northcentralusstage.k4apps.io/admin/functions/QueueTrigger",
            "invoke_url_template": null,
            "language": "node",
            "config": {
                "bindings": [
                    {
                        "name": "myQueueItem",
                        "type": "queueTrigger",
                        "direction": "in",
                        "queueName": "js-queue-items",
                        "connection": "AzureWebJobsStorage"
                    }
                ]
            },
            "files": null,
            "test_data": "",
            "isDisabled": false,
            "isDirect": false,
            "isProxy": false
        }
    ]
}

I create a transformation logic simply convert from the triggers.

Durable Functions / Logic Apps Usage requires host.json information

Durable Functions / Logic Apps Transformation requires host.json information. It is ok for zip deploy, however, for sync trigger scenario with container app, there is no host.json information on the SyncTrigger payload.
We need to update the Azure Functions Host for enabling it.

JObject storageProviderConfig = hostJson.SelectToken(durableStorageProviderPath) as JObject;

Logging

Currently, I use Console.WriteLine for the debugging. However, we should use ITracer However, it doesn't show something. Do we need to configure it? I'll remove the Console.WriteLine before migrating to the PullRequest.

@cgillum
Copy link
Collaborator

cgillum commented May 18, 2021

Not just Durable Functions - Logic Apps also requires host.json: https://github.com/Azure-App-Service/KuduLite/pull/196/files

@TsuyoshiUshio
Copy link
Collaborator Author

Thank you @cgillum I updated the description.

//Auth header value is null or empty return false
var funcAppAuthToken = siteTokenHeaderValue.FirstOrDefault();
if (string.IsNullOrEmpty(funcAppAuthToken))
{
return false;
}

Console.WriteLine("**** SyncTriggerAuthenticator funcAppAuthToken ***");
Copy link
Collaborator

Choose a reason for hiding this comment

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

are these Console.WriteLine needed? Can they use a logger of some sort?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you @ahmelsayed This line is just for temporary debugging, However, I'd like to use Logger.
I found ITracer is injected, so that I tried to use it, However, it doesn't show anything on the console log. Do you know why it doesn't show the console log? Do we need to configure for it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Are you referring to ILogger ? If yes, you need to explicitly add console logging provider - https://docs.microsoft.com/en-us/aspnet/core/fundamentals/logging/?view=aspnetcore-5.0

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As we discussed on the standup, I remove the Console logs.
I'll update the logs for production on this issue. #202

@pragnagopa
Copy link
Collaborator

Host issue: Azure/azure-functions-host#7382

@TsuyoshiUshio TsuyoshiUshio marked this pull request as ready for review May 19, 2021 18:28
@TsuyoshiUshio
Copy link
Collaborator Author

Hi @pragnagopa @ahmelsayed
I update the PR and remove the draft.
I tested on the latest build and see the transformation happens when I send a rest request from a function host container.

TODO:
For the fix of the Durable Functions, I'll create a new PR after I create a PR of the functions host first.

@TsuyoshiUshio
Copy link
Collaborator Author

Hi @ahmelsayed
I update the API to the POST. Looks good?

@ahmelsayed
Copy link
Collaborator

Does it not need to change in Startup.cs? if this works, then it's fine, I'm not sure what order aspnet core loads these in.

@TsuyoshiUshio
Copy link
Collaborator Author

HI @ahmelsayed
Good point. I'll change it as well and test it if it works.

@TsuyoshiUshio
Copy link
Collaborator Author

Hi @ahmelsayed
I tested this branch and send POST request. I can see the transformation by this request from Azure Functions Host pod.

image

@pragnagopa pragnagopa changed the title Fix: SyncTrigger issue Fix: [Functions][Kubernetes]SyncTrigger issue May 19, 2021
Copy link
Collaborator

@pragnagopa pragnagopa left a comment

Choose a reason for hiding this comment

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

👍

@pragnagopa pragnagopa merged commit 96aae86 into Azure-App-Service:feature/k8se May 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants