Skip to content

Enhance encryption configuration to enforce required keys for symmetric encryption#175

Merged
AqeelMuhammad merged 1 commit intowso2:mainfrom
AqeelMuhammad:feature/AES256
Mar 19, 2026
Merged

Enhance encryption configuration to enforce required keys for symmetric encryption#175
AqeelMuhammad merged 1 commit intowso2:mainfrom
AqeelMuhammad:feature/AES256

Conversation

@AqeelMuhammad
Copy link
Copy Markdown
Contributor

@AqeelMuhammad AqeelMuhammad commented Mar 19, 2026

Purpose

This pull request updates the encryption configuration logic across multiple deployment configuration files to improve the handling of symmetric key encryption, especially when secure vault is enabled. The changes ensure that encryption keys are required and validated appropriately, and that secrets are only set when the symmetric key provider is used.

Key changes include:

Encryption Key Validation and Configuration Logic:

  • Refactored the conditional logic for setting the encryption key in the [encryption] section, so that checks for the symmetric key provider and secure vault are clearer and more robust. Now, the requirement for an encryption key is enforced only when the SymmetricKeyInternalCryptoProvider is selected, and the error messages are more precise. [1] [2] [3] [4] [5] [6] [7] [8]

Secrets Section Updates:

  • Updated the [secrets] section to only include the encryption_key secret when the symmetric key provider is in use, preventing unnecessary secret population for other providers. [1] [2] [3] [4] [5] [6] [7] [8]

These improvements enhance security and reduce configuration errors related to encryption key management in all-in-one, distributed control plane, gateway, key manager, and traffic manager deployments.

Summary by CodeRabbit

  • Refactor
    • Simplified encryption configuration validation logic across deployment templates for improved consistency.
    • Streamlined secure vault encryption key handling with more direct validation checks.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 19, 2026

Walkthrough

Refactored encryption key validation logic across deployment templates to simplify symmetric provider conditions, remove redundant checks in secureVault branches, and conditionally emit encryption_key in secrets only when using the symmetric crypto provider. Changes follow a consistent pattern across all modified files.

Changes

Cohort / File(s) Summary
All-in-One Deployments
all-in-one/confs/instance-1/deployment.toml, all-in-one/confs/instance-2/deployment.toml
Simplified encryption key validation by removing redundant provider equality checks within secureVault branches. Changed symmetric-encryption failure logic to use default else branches instead of provider-specific conditions. Made encryption_key emission in secrets conditional on symmetric provider detection.
Distributed Control Plane
distributed/control-plane/confs/instance-1/deployment.toml, distributed/control-plane/confs/instance-2/deployment.toml
Refactored [encryption] section logic to trim and validate encryption.key emptiness independently of nested provider checks. Generalized failure conditions from provider-gated branches to unconditional fallback paths. Scoped encryption_key secret emission to symmetric provider only.
Distributed Gateway & Key Manager
distributed/gateway/confs/deployment.toml, distributed/key-manager/confs/deployment.toml
Removed nested provider dependencies from key validation and selection logic. Simplified control flow by replacing provider-specific else-if branches with generic else paths. Conditionally populated encryption_key based on symmetric provider presence rather than secureVault enablement alone.
Distributed Traffic Manager
distributed/traffic-manager/confs/instance-1/deployment.toml, distributed/traffic-manager/confs/instance-2/deployment.toml
Restructured symmetric provider validation to check encryption.key emptiness directly without redundant provider re-checks. Modified fallback failure conditions from provider-specific to broad else branches. Limited encryption_key rendering in secrets to symmetric provider cases only.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A tidy hop through config land,
Where symmetric keys now firmly stand—
Conditions trimmed with careful paws,
Redundant checks removed without flaws.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description provides Purpose, Goals, and Approach sections with specific details and references; however, several required template sections (User stories, Release note, Documentation, Training, Certification, Marketing, Testing, Security checks, Samples, Related PRs, Migrations, Test environment, and Learning) are absent or not properly filled. Complete the PR description by adding the missing required sections from the template, particularly User stories, Release note, Documentation, Security checks, and Testing details (Unit tests and Integration tests coverage information).
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main enhancement: refactoring encryption key validation logic to enforce required keys specifically for symmetric encryption providers.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@distributed/gateway/confs/deployment.toml`:
- Around line 284-304: The fallback fail message inside the symmetric-encryption
Helm template is misleading: update the fail text emitted by the conditional
that checks .Values.wso2.apim.configurations.encryption.key /
.Values.wso2.apim.configurations.existingSecret.* to explicitly state that
secureVault also requires providing wso2.apim.configurations.encryption.key (or
the existingSecret keys) because the same key is later used to populate
[secrets].encryption_key; locate the conditional block using the symbols
.Values.wso2.apim.configurations.encryption.key,
.Values.wso2.apim.configurations.existingSecret.secretName,
.Values.wso2.apim.configurations.existingSecret.encryptionKeyKey,
.Values.wso2.apim.secureVaultEnabled and change the fail message to mention that
secureVault alone is not sufficient and an encryption key must be supplied.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 49c83e1b-3ab9-436d-a996-558d0acd956c

📥 Commits

Reviewing files that changed from the base of the PR and between c74d258 and a4a9296.

📒 Files selected for processing (8)
  • all-in-one/confs/instance-1/deployment.toml
  • all-in-one/confs/instance-2/deployment.toml
  • distributed/control-plane/confs/instance-1/deployment.toml
  • distributed/control-plane/confs/instance-2/deployment.toml
  • distributed/gateway/confs/deployment.toml
  • distributed/key-manager/confs/deployment.toml
  • distributed/traffic-manager/confs/instance-1/deployment.toml
  • distributed/traffic-manager/confs/instance-2/deployment.toml

Comment thread distributed/gateway/confs/deployment.toml
@AqeelMuhammad AqeelMuhammad merged commit b678115 into wso2:main Mar 19, 2026
2 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.

2 participants