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

Modify implementation to allow an API and a Common policies with same and version #12660

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

RusJaI
Copy link
Contributor

@RusJaI RusJaI commented Oct 13, 2024

Public fix of https://github.com/wso2-enterprise/wso2-apim-internal/issues/7352

Previously when exporting an API which has an API policy (ex :policy1) and common policy (ex : policy1) with the same name and version, only one of them is exported. The import flow also didn't allow to import those policies. The PR fixes the above.

@RusJaI RusJaI changed the title Modify implementation to allow an API and a Common policies with same and version [Do not Merge ] Modify implementation to allow an API and a Common policies with same and version Oct 15, 2024
@RusJaI RusJaI marked this pull request as draft October 15, 2024 11:56
@RusJaI RusJaI changed the title [Do not Merge ] Modify implementation to allow an API and a Common policies with same and version Modify implementation to allow an API and a Common policies with same and version Oct 15, 2024
@RusJaI RusJaI force-pushed the master branch 4 times, most recently from 7b1a258 to 0a6e19d Compare December 20, 2024 16:55
@RusJaI RusJaI marked this pull request as ready for review December 24, 2024 20:18
if (policyData != null) {
policySpec = policyData.getSpecification();
appliedPolicy.setPolicyId(policyData.getPolicyId());
if (policyType == null || ImportExportConstants.POLICY_TYPE_API.equalsIgnoreCase(policyType)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we check null or api type here? Also, don't we have to consider the else case here?

if (policyData != null) {
policySpec = policyData.getSpecification();
appliedPolicy.setPolicyId(policyData.getPolicyId());
if (policyType == null || ImportExportConstants.POLICY_TYPE_COMMON.equalsIgnoreCase(policyType)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here. Why do we have the null or common check and no else conditions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if policy is not found in the project, policy can be referenced from an existing policy. That logic is performed here.
(When importing an API which has been exported from a previous APIM version which doesn't have policy support, the default policy type is set to null.)
In the process of finding policy among existing policies :

  1. check if the policy is within API Specific operational policies. this check is made for the policies which the type is 'api' or not defined which is filtered by null check
  2. check if the policy is within Common operational policies. this check is made for the policies which the type is 'common' or not defined which is filtered by null check

Comment on lines +842 to +847
if (policyType == null
|| ImportExportConstants.POLICY_TYPE_API.equalsIgnoreCase(policyType)) {
policyData = provider.getAPISpecificOperationPolicyByPolicyName(policy.getPolicyName(),
policy.getPolicyVersion(), api.getUuid(), null,
tenantDomain, false);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a comment to explain why we do this null or 'api' check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this method is used to validate the provided policy list and return the validated list. if the policy spec is not there, then it checks whether the policy has been referenced. if the policy type is not defined or defined as an api level policy, it fetches the policy data. If the policyData is null , which means there's no API Specific policy with the provided details. Then it's checked within common policies.

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.

2 participants