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 the implementation of Gateway-Specific Global-Level Policies Feature #12201

Merged
merged 51 commits into from
Jan 5, 2024

Conversation

YasasRangika
Copy link
Contributor

@YasasRangika YasasRangika commented Dec 13, 2023

Purpose

To add the implementation of functionality to enable or disable Gateway Specific Global Level Policies using both the user interface (UI) and Representational State Transfer (REST) APIs.

Problem Addressed:
In earlier versions of API Manager releases, adding a gateway-level policy to a gateway node was a manual and error-prone process. Users were required to create XML-type policy files themselves and place them in the designated directory: <APIM_GATEWAY_NODE_HOME>/repository/deployment/server/synapse-configs/default/sequences, following a specific naming pattern of WSO2AM--Ext--<DIRECTION>. This feature enables users to define and manage such policies more conveniently using the provided user interface or REST APIs, eliminating the need for manual XML creation and placement.

Goal

Fixes: wso2/api-manager#1710

Approach

The feature revolves around the concept of policy mappings, allowing privileged users to seamlessly create, apply, and deploy global-level policies to the gateways. This process facilitates policy management, similar to the way API revisions are created and deployed in the “Deployments” tab. By creating policy mappings for different request, response, or fault flows, users can apply these mappings to gateways and deploy them with ease.

Furthermore, this new feature does not affect the previously manually deployed sequences following the new naming pattern: WSO2AMGW--Ext--<DIRECTION>. These sequences execute after the sequences named WSO2AM--Ext--<DIRECTION>.

@YasasRangika
Copy link
Contributor Author

YasasRangika commented Dec 20, 2023

Important:

After investigation, I've pinpointed the root cause of the failed test cases:

2023-12-14T20:09:50.6760296Z [ERROR] Failures: 
2023-12-14T20:09:50.6761350Z [ERROR] org.wso2.am.integration.tests.api.lifecycle.CustomLifeCycleTestCase.initialize
2023-12-14T20:09:50.6763043Z [ERROR]   Run 1: CustomLifeCycleTestCase.initialize:84 » Api 
2023-12-14T20:09:50.6763869Z [INFO]   Run 2: PASS
2023-12-14T20:09:50.6764310Z [INFO] 
2023-12-14T20:09:50.6765551Z [ERROR] org.wso2.am.integration.tests.other.AdvancedConfigurationsTestCase.testUpdateTenantConfiguration
2023-12-14T20:09:50.6769544Z [ERROR]   Run 1: AdvancedConfigurationsTestCase.testUpdateTenantConfiguration:65 » Api 
2023-12-14T20:09:50.6771239Z [ERROR]   Run 2: AdvancedConfigurationsTestCase.testUpdateTenantConfiguration:65 » Api 
2023-12-14T20:09:50.6772143Z [INFO] 
2023-12-14T20:09:50.6773323Z [ERROR] org.wso2.am.integration.tests.publisher.GetLinterCustomRulesThroughThePublisherRestAPITestCase.setEnvironment
2023-12-14T20:09:50.6775148Z [ERROR]   Run 1: GetLinterCustomRulesThroughThePublisherRestAPITestCase.setEnvironment:88 » Api
2023-12-14T20:09:50.6776654Z [ERROR]   Run 2: GetLinterCustomRulesThroughThePublisherRestAPITestCase.setEnvironment:88 » Api

The issue arose due to the introduction of new scopes into the backend by this PR. During the validation of scopes using the test class's methods calling restAPIAdmin.updateTenantConfig, the backend test module operates with a separate tenant-conf file, resulting in discrepancies. These scope changes within the APIs of carbon-apimgt have caused the failures. I confirm that I ran some tests locally after making manual adjustments to the tenant-conf file, and everything worked fine.

Due to this, we have to proceed with merging the PRs despite these errors. To prevent breaking other tests, we'll ensure the local tests pass before proceeding with the merge of product test changes.

Map<String, Set<String>> gatewaysToAdd = new HashMap<>();
Map<String, Set<String>> gatewaysToRemove = new HashMap<>();

for (boolean isDeployment : gatewayPolicyDeploymentMap.keySet()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can gatewayPolicyDeploymentMap be null in any case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The gatewayPolicyDeploymentMap will never be null under any circumstance. This is ensured during the initial assignment of values to the gatewayPolicyDeploymentMap, where validation occurs to check if the OperationPolicyData is empty in GatewayPoliciesApiServiceImpl. If it is not empty, the values are obtained from it using the method GatewayPolicyMappingUtil.fromDTOToGatewayPolicyDeploymentMap. However, in cases where there are no new values assigned for the gateway deployments, it will return an empty ArrayList object.

public String updateGatewayGlobalPolicies(List<OperationPolicy> gatewayGlobalPolicyList, String description,
String name, String orgId, String policyMappingId) throws APIManagementException {
List<OperationPolicy> policyList = apiMgtDAO.getGatewayPoliciesOfPolicyMapping(policyMappingId);
if (policyList.isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can policyList be null any case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it will return an empty ArrayList even if no elements are retrieved from the database.

@YasasRangika
Copy link
Contributor Author

Hi All,

The API Manager Build with Test / build-product (1, group1) and API Manager Build with Test / build-product (2, group2) consistently fail in various test rounds with different test cases. Initially, the tests failed due to missing data from tenant-conf.json, which Sahan addressed with PR [1]. It seems that there are intermittent failures within the test suite, where certain test cases fail in one run but pass in subsequent runs, indicating intermittent issues rather than consistent failures.

While the identical set of test cases consistently failed twice in consideration for group 2, suggesting a persistent or recurring issue rather than an intermittent one, a review of the logs did not reveal any issues directly linked to changes in this feature's implementation.

[1] wso2/product-apim#13313

Thanks and Regards,
Yasas

@dushaniw dushaniw merged commit cfa8a1a into wso2:master Jan 5, 2024
3 of 7 checks passed
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.

Support Deploy/Undeploy Global Level Policies from UI/REST APIs
4 participants