Skip to content

Add rulesets#15

Merged
dmezhensky merged 2 commits intomainfrom
fix/add-rulesets
Nov 12, 2025
Merged

Add rulesets#15
dmezhensky merged 2 commits intomainfrom
fix/add-rulesets

Conversation

@dmezhensky
Copy link
Copy Markdown
Collaborator

@dmezhensky dmezhensky commented Nov 12, 2025

Summary by CodeRabbit

  • New Features

    • Firewall rulesets now automatically initialize and return created rules when not found
    • Redirect rulesets now automatically initialize and return created rules when not found
  • Bug Fixes

    • Corrected error status reporting for redirect ruleset creation failures
  • Chores

    • Bumped package version to v1.0.14

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Nov 12, 2025

Walkthrough

Added 404-response handling to getFirewallRules() and getRedirectRules() in cloudflare.js: on 404 the code now creates the missing ruleset, parses the creation response, validates status, extracts the ruleset ID and rules, and returns them. Also bumped package version in package.json.

Changes

Cohort / File(s) Summary
Ruleset 404-handling logic
cloudflare.js
On 404 from ruleset GET, create a new ruleset, parse and validate the creation response, extract and return the new ruleset ID and rules. Fixed a redirect-ruleset error message to use the correct createStatusCode variable.
Package metadata bump
package.json
Version updated from 1.0.13 to 1.0.14.

Sequence Diagram

sequenceDiagram
    participant Client
    participant CloudFlare
    participant RulesetAPI as Ruleset API

    Client->>CloudFlare: getFirewallRules() / getRedirectRules()
    CloudFlare->>RulesetAPI: GET ruleset

    alt 200 OK
        RulesetAPI-->>CloudFlare: 200 + existing ruleset data
        CloudFlare-->>Client: return ruleset ID & rules
    else 404 Not Found
        CloudFlare->>RulesetAPI: POST create new ruleset
        RulesetAPI-->>CloudFlare: Creation response (status + body)
        rect rgb(220, 235, 255)
            CloudFlare->>CloudFlare: parse response<br/>validate status (createStatusCode)<br/>extract ruleset ID & rules
        end
        CloudFlare-->>Client: return new ruleset ID & rules
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Single primary file with parallel logic applied to two methods plus a small metadata bump.
  • Pay attention to: parsing/validation branches in both 404 flows, correct use of createStatusCode in error messages, and that returned payload shapes match callers' expectations.

Possibly related PRs

Suggested reviewers

  • nosovk

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add rulesets' directly relates to the main changes in the PR, which add firewall ruleset and redirect ruleset initialization logic to cloudflare.js when 404 responses occur.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/add-rulesets

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8d1e1be and 620637c.

📒 Files selected for processing (1)
  • package.json (1 hunks)
🔇 Additional comments (1)
package.json (1)

3-3: Verify semantic versioning appropriateness for the version bump.

The version is bumped from 1.0.13 to 1.0.14 (patch level), which is appropriate for backwards-compatible bug fixes. However, the PR summary indicates added 404-response handling and ruleset creation logic, which could be interpreted as new functionality and might warrant a MINOR version bump (1.1.0) instead. Please confirm whether this is a bug fix (to previously broken/missing handling) or a new feature addition to determine if the bump semantics are correct.


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: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
cloudflare.js (1)

384-392: Fix variable references and add defensive parsing.

This code has multiple issues:

  1. Line 387: Uses statusCode instead of createStatusCode in the error message, which would incorrectly report "404" instead of the actual creation failure status.
  2. Line 384: Missing try-catch for JSON parsing, unlike the consistent pattern in getFirewallRules() (lines 233-237).
  3. Line 392: References response instead of createResponse in the error message.

Apply this diff to fix all issues:

-      const createResponse = await createBody.json()
+      let createResponse
+      try {
+        createResponse = await createBody.json()
+      } catch (e) {
+        createResponse = await createBody.text()
+      }

       if (createStatusCode !== 200) {
-        throw new Error(`Could not create redirect ruleset: ${statusCode}, error: ${JSON.stringify(createResponse)}`)
+        throw new Error(`Could not create redirect ruleset: ${createStatusCode}, error: ${JSON.stringify(createResponse)}`)
       }

       const { id, rules } = createResponse?.result ?? {}
       if (!id) {
-        throw new Error(`Could not get redirect rules ruleset ID: got ${id}, received value: ${JSON.stringify(response)}`)
+        throw new Error(`Could not get redirect rules ruleset ID: got ${id}, received value: ${JSON.stringify(createResponse)}`)
       }
🧹 Nitpick comments (1)
cloudflare.js (1)

213-249: LGTM! Consider extracting common ruleset creation logic.

The 404 handling correctly creates a new ruleset when none exists. The try-catch for JSON parsing and validation logic are well-implemented.

However, this logic is nearly identical to the 404 handling in getRedirectRules() (lines 364-395). Consider extracting a common helper method to reduce duplication:

async createRuleset(name, phase) {
  const url = CLOUDFLARE_API_URL + `zones/${this.zoneId}/rulesets`;
  const payload = {
    name,
    kind: 'zone',
    phase,
    rules: []
  };

  const { statusCode, body } = await this.requestWithDelay(url, {
    method: 'POST',
    headers: {
      ...this.authorizationHeaders,
      'Content-Type': 'application/json'
    },
    body: JSON.stringify(payload)
  });

  let response;
  try {
    response = await body.json();
  } catch (e) {
    response = await body.text();
  }

  if (statusCode !== 200) {
    throw new Error(`Could not create ${phase} ruleset: ${statusCode}, error: ${JSON.stringify(response)}`);
  }

  const { id, rules } = response?.result ?? {};
  if (!id) {
    throw new Error(`Could not get ${phase} ruleset ID: got ${id}, received value: ${JSON.stringify(response)}`);
  }

  return { id, rules: rules ?? [] };
}

Then use it in both methods:

if (statusCode === 404) {
  console.log('Firewall ruleset was not found. Initializing firewall ruleset creation...');
  return await this.createRuleset('Custom firewall ruleset', 'http_request_firewall_custom');
}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 35bf8fd and 8d1e1be.

📒 Files selected for processing (1)
  • cloudflare.js (1 hunks)

@dmezhensky dmezhensky merged commit 8a6ae80 into main Nov 12, 2025
1 check passed
@dmezhensky dmezhensky deleted the fix/add-rulesets branch November 12, 2025 14:08
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.

1 participant