Add support for HeaderModifier filter in Config Deployer #2381

sgayangi · 2024-05-29T06:22:36Z

Purpose

This PR adds the config deployer level support for the HeaderModifier filter, as detailed in #2378

Sample apk-conf file would be

id: "f7996dce4ac15e2af0f8ee14546c4f72988eddae"
name: "EmployeeServiceAPI"
basePath: "/test"
version: "3.14"
type: "REST"
defaultVersion: false
endpointConfigurations:
  production:
    endpoint: "https://httpbin.org/anything"
operations:
  - target: "/employee"
    verb: "GET"
    secured: false
    scopes: []
    operationPolicies:
      request:
        - policyName: AddHeaders
          policyVersion: v1
          parameters:
            headers:
              - name: "Test-Request-Header"
                value: "Test-Value"
        - policyName: SetHeaders
          policyVersion: v1
          parameters:
            headers:
              - name: "Set-Request-Header"
                value: "Test-Value"
        - policyName: RemoveHeaders
          policyVersion: v1
          parameters:
            headers:
              - "X-Test-Header"
  - target: "/employee"
    verb: "POST"
    secured: true
    scopes: []
  - target: "/employee/{employeeId}"
    verb: "PUT"
    secured: true
    scopes: []
  - target: "/employee/{employeeId}"
    verb: "DELETE"
    secured: true
    scopes: []

ShammiL · 2024-06-04T05:20:50Z

runtime/config-deployer-service/ballerina/APIClient.bal

+                if policy.policyName == "AddHeaders" {
+                    ModifierHeader[] addHeaders = <ModifierHeader[]>policyParameters.headers;
+                    foreach ModifierHeader header in addHeaders {
+                        addPolicies.push(header);
+                    }
+                }
+                if policy.policyName == "SetHeaders" {
+                    ModifierHeader[] setHeaders = <ModifierHeader[]>policyParameters.headers;
+                    foreach ModifierHeader header in setHeaders {
+                        setPolicies.push(header);
+                    }
                }
-                if (policyName == "removeHeader") {
-                    string httpHeader = <string>policyParameters.get("headerName");
-                    removePolicies.push(httpHeader);
+                if policy.policyName == "RemoveHeaders" {


You can use match here - https://ballerina.io/learn/by-example/match-statement/

Fixed in 2f908b5

ShammiL · 2024-06-04T05:21:56Z

runtime/config-deployer-service/ballerina/APIClient.bal

+            if requestPolicies is APKOperationPolicy[] && requestPolicies.length() > 0 {
+                model:HTTPRouteFilter headerModifierFilter = {'type: "RequestHeaderModifier"};
+                headerModifierFilter.requestHeaderModifier = self.extractHttpHeaderFilterData(requestPolicies, organization);
+                routeFilters.push(headerModifierFilter);


Since this line is repeated inside both if blocks, you could move it out

Since the filter type (RequestHeaderModifier and ResponseHeaderModifier) and the properties added are different in each if block, I left it as is.

gayaldassanayake · 2024-06-04T05:24:11Z

runtime/config-deployer-service/ballerina/APIClient.bal

+                    model:HTTPRouteRule httpRouteRule = {
+                        matches: self.retrieveHTTPMatches(apkConf, operation, organization),
+                        backendRefs: self.retrieveGeneratedBackend(apkConf, endpointToUse, endpointType),
+                        filters: self.generateFilters(apiArtifact, apkConf, endpointToUse, operation, endpointType, organization)
+                    };


We can remove the else block here (and in the code lines above) to reduce the indentation depth. look for other similar places as well

if apkConf.'type == API_TYPE_GRAPHQL { model:GQLRouteMatch[]|error routeMatches = self.retrieveGQLMatches(apkConf, operation, organization); if routeMatches is model:GQLRouteMatch[] && routeMatches.length() > 0 { model:GQLRouteRule gqlRouteRule = {matches: routeMatches}; return gqlRouteRule; } return e909022("Provided Type currently not supported for GraphQL APIs.", error("Provided Type currently not supported for GraphQL APIs.")); } model:HTTPRouteRule httpRouteRule = { matches: self.retrieveHTTPMatches(apkConf, operation, organization), backendRefs: self.retrieveGeneratedBackend(apkConf, endpointToUse, endpointType), filters: self.generateFilters(apiArtifact, apkConf, endpointToUse, operation, endpointType, organization) }; return httpRouteRule;

Since more API types will be added in the future, I instead modified the else block to be an if else block.

if apkConf.'type == API_TYPE_GRAPHQL { model:GQLRouteMatch[]|error routeMatches = self.retrieveGQLMatches(apkConf, operation, organization); if routeMatches is model:GQLRouteMatch[] && routeMatches.length() > 0 { model:GQLRouteRule gqlRouteRule = {matches: routeMatches}; return gqlRouteRule; } else { return e909022("Provided Type currently not supported for GraphQL APIs.", error("Provided Type currently not supported for GraphQL APIs.")); } } else if apkConf.'type == API_TYPE_REST { { model:HTTPRouteRule httpRouteRule = { matches: self.retrieveHTTPMatches(apkConf, operation, organization), backendRefs: self.retrieveGeneratedBackend(apkConf, endpointToUse, endpointType), filters: self.generateFilters(apiArtifact, apkConf, endpointToUse, operation, endpointType, organization) }; return httpRouteRule; } } else { return e909018("Invalid API Type specified"); }

gayaldassanayake · 2024-06-04T05:26:31Z

runtime/config-deployer-service/ballerina/APIClient.bal

                }
            }
        }
        model:HTTPHeaderFilter headerModifier = {};
+        if (addPolicies != []) {


Suggested change

if (addPolicies != []) {

if addPolicies != [] {

Fixed in 2f908b5

gayaldassanayake · 2024-06-04T05:29:32Z

runtime/config-deployer-service/ballerina/APIClient.bal

+                    } else if (policyName == "AddHeaders" || policyName == "SetHeaders" || policyName == "RemoveHeaders") {
+
                    } else {
                        return e909052(error("Incorrect API Policy name provided."));
                    }


Suggested change

} else if (policyName == "AddHeaders" || policyName == "SetHeaders" || policyName == "RemoveHeaders") {

} else {

return e909052(error("Incorrect API Policy name provided."));

}

} else if (policyName != "AddHeaders" && policyName != "SetHeaders" && policyName != "RemoveHeaders") {

return e909052(error("Incorrect API Policy name provided."));

}

Fixed in 2f908b5

Thevakumar-Luheerathan · 2024-06-04T05:33:34Z

runtime/config-deployer-service/ballerina/APIClient.bal

@@ -1228,6 +1260,8 @@ public class APIClient {
                        model:BackendJWT backendJwt = self.retrieveBackendJWTPolicy(apkConf, apiArtifact, backendJWTPolicy, operations, organization);
                        apiArtifact.backendJwt = backendJwt;
                        policyReferences.push(<model:BackendJwtReference>{name: backendJwt.metadata.name});
+                    } else if (policyName == "AddHeaders" || policyName == "SetHeaders" || policyName == "RemoveHeaders") {


Brackets are unnecessary. We can remove them.

Fixed in 2f908b5

gayaldassanayake · 2024-06-04T05:34:30Z

runtime/config-deployer-service/ballerina/types.bal

@@ -202,6 +202,11 @@ public type Authentication record {|
    boolean enabled = true;
 |};

+public type HeaderModifierPolicy record {


Let's add documentation comments for all public records

Fixed in 2f908b5

Thevakumar-Luheerathan · 2024-06-04T05:34:31Z

runtime/config-deployer-service/ballerina/APIClient.bal

+                        addPolicies.push(header);
+                    }
+                }
+                if policy.policyName == "SetHeaders" {


We may use enum for the policyName.

Fixed in 2f908b5

Thevakumar-Luheerathan · 2024-06-04T05:37:54Z

runtime/config-deployer-service/ballerina/APIClient.bal

                }
            }
        }
        model:HTTPHeaderFilter headerModifier = {};
+        if (addPolicies != []) {
+            headerModifier.add = addPolicies;
+        }
        if (setPolicies != []) {


Remove brackets here.

Fixed in 2f908b5

sgayangi added the trigger-action When this includes apk actions getting trigger for pr label May 29, 2024

Add support for HeaderModifier filter in config deployer

a498253

sgayangi force-pushed the #2378-httproute-filters branch from 0234576 to a498253 Compare May 29, 2024 09:17

github-actions bot removed the trigger-action When this includes apk actions getting trigger for pr label May 29, 2024

Add cucumber test for header modifier filter

1d61ce4

sgayangi added trigger-action When this includes apk actions getting trigger for pr and removed trigger-action When this includes apk actions getting trigger for pr labels May 29, 2024

sgayangi requested a review from AmaliMatharaarachchi May 30, 2024 03:48

ShammiL reviewed Jun 4, 2024

View reviewed changes

gayaldassanayake reviewed Jun 4, 2024

View reviewed changes

Thevakumar-Luheerathan reviewed Jun 4, 2024

View reviewed changes

gayaldassanayake reviewed Jun 4, 2024

View reviewed changes

Thevakumar-Luheerathan reviewed Jun 4, 2024

View reviewed changes

github-actions bot removed the trigger-action When this includes apk actions getting trigger for pr label Jun 4, 2024

sgayangi force-pushed the #2378-httproute-filters branch 2 times, most recently from d0e33c3 to 3e75dbe Compare June 4, 2024 08:47

sgayangi added the trigger-action When this includes apk actions getting trigger for pr label Jun 4, 2024

Address review comments

2f908b5

sgayangi force-pushed the #2378-httproute-filters branch from 3e75dbe to 2f908b5 Compare June 4, 2024 09:00

github-actions bot removed the trigger-action When this includes apk actions getting trigger for pr label Jun 4, 2024

sgayangi added trigger-action When this includes apk actions getting trigger for pr and removed trigger-action When this includes apk actions getting trigger for pr labels Jun 4, 2024

Add propagation to APIM CP

cf4675c

github-actions bot removed the trigger-action When this includes apk actions getting trigger for pr label Jun 6, 2024

sgayangi added the trigger-action When this includes apk actions getting trigger for pr label Jun 6, 2024

sgayangi mentioned this pull request Jun 6, 2024

Add header modifier, request redirect and request mirroring support in Agent #2387

Closed

sgayangi added trigger-action When this includes apk actions getting trigger for pr and removed trigger-action When this includes apk actions getting trigger for pr labels Jun 6, 2024

CrowleyRajapakse approved these changes Jun 18, 2024

View reviewed changes

CrowleyRajapakse merged commit 3115de5 into wso2:main Jun 18, 2024
21 of 24 checks passed

github-actions bot removed the trigger-action When this includes apk actions getting trigger for pr label Jun 18, 2024

sgayangi mentioned this pull request Jun 18, 2024

Support for Gateway API supported filters for HTTPRoutes #2378

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for HeaderModifier filter in Config Deployer #2381

Add support for HeaderModifier filter in Config Deployer #2381

sgayangi commented May 29, 2024 •

edited

Loading

ShammiL Jun 4, 2024

sgayangi Jun 4, 2024

ShammiL Jun 4, 2024

sgayangi Jun 4, 2024

gayaldassanayake Jun 4, 2024 •

edited

Loading

sgayangi Jun 4, 2024 •

edited

Loading

gayaldassanayake Jun 4, 2024

sgayangi Jun 4, 2024 •

edited

Loading

gayaldassanayake Jun 4, 2024

sgayangi Jun 4, 2024 •

edited

Loading

Thevakumar-Luheerathan Jun 4, 2024

sgayangi Jun 4, 2024 •

edited

Loading

gayaldassanayake Jun 4, 2024

sgayangi Jun 4, 2024 •

edited

Loading

Thevakumar-Luheerathan Jun 4, 2024

sgayangi Jun 4, 2024 •

edited

Loading

Thevakumar-Luheerathan Jun 4, 2024

sgayangi Jun 4, 2024 •

edited

Loading

Add support for HeaderModifier filter in Config Deployer #2381

Add support for HeaderModifier filter in Config Deployer #2381

Conversation

sgayangi commented May 29, 2024 • edited Loading

Purpose

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gayaldassanayake Jun 4, 2024 • edited Loading

Choose a reason for hiding this comment

sgayangi Jun 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sgayangi Jun 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sgayangi Jun 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sgayangi Jun 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sgayangi Jun 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sgayangi Jun 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sgayangi Jun 4, 2024 • edited Loading

Choose a reason for hiding this comment

sgayangi commented May 29, 2024 •

edited

Loading

gayaldassanayake Jun 4, 2024 •

edited

Loading

sgayangi Jun 4, 2024 •

edited

Loading

sgayangi Jun 4, 2024 •

edited

Loading

sgayangi Jun 4, 2024 •

edited

Loading

sgayangi Jun 4, 2024 •

edited

Loading

sgayangi Jun 4, 2024 •

edited

Loading

sgayangi Jun 4, 2024 •

edited

Loading

sgayangi Jun 4, 2024 •

edited

Loading