Skip to content

Enforce existence of the bundle manifest file (fixes #688) - #695

Open
EngineerOnTravel wants to merge 1 commit into
apigee:mainfrom
EngineerOnTravel:fix-issue-688
Open

Enforce existence of the bundle manifest file (fixes #688)#695
EngineerOnTravel wants to merge 1 commit into
apigee:mainfrom
EngineerOnTravel:fix-issue-688

Conversation

@EngineerOnTravel

Copy link
Copy Markdown

Enforce existence of the bundle manifest file (fixes #688)

Problem

apigeelint never validates that a bundle actually has a manifest file
(apiproxy.xml / sharedflowbundle.xml) at its root.

In lib/package/Bundle.js, processFileSystem() reads the .xml files at
the bundle root and only guards against the "too many" case:

if (files.length > 1) {
  throw new Error("There is more than one .xml file found at the bundle root");
}

bundle.filePath = bundle.root + "/" + files[0];

When files.length === 0 (no manifest file at all), files[0] is
undefined, so bundle.filePath silently becomes the string
"<root>/undefined". Every downstream consumer of filePath
(getElement(), getName(), getRevision(), etc.) just calls
fs.existsSync(...), finds nothing, and quietly falls back to defaults.
The result: a bundle with a completely missing manifest is linted as if
it were fine, and no error or warning is ever produced, even though such
a bundle cannot be imported into Apigee at all.

Fix

  • lib/package/Bundle.js: only set bundle.filePath when a manifest
    file was actually found; leave it undefined otherwise (instead of the
    bogus "<root>/undefined" path).
  • Added a new rule, BN015 (lib/package/plugins/BN015-checkManifestExists.js),
    that runs at the Bundle level and reports a severity-2 (error) message
    when bundle.filePath is undefined, i.e. when no manifest file was
    found at the bundle root. This follows the same pattern as the other
    BN0xx bundle-structure rules (e.g. BN001, BN010), so it plugs into
    the existing plugin discovery, reporting, and severity/exit-code
    machinery for free.
  • Updated the rule table in README.md to document BN015.
  • Added test/fixtures/resources/missingManifest/apiproxy/ — a bundle
    fixture with a valid apiproxy folder structure (proxies/default.xml)
    but no manifest .xml file at the root — to exercise the new behavior.
  • Added test/specs/BN015-checkManifestExists.js, verifying:
    • a bundle missing its manifest file produces exactly one BN015
      error-severity message;
    • a normal, well-formed bundle (sampleProxy/24Solver/apiproxy)
      produces no BN015 message.
  • Added a small assertion to test/specs/testBundleEx.js confirming
    bundle.filePath is undefined (not ".../undefined") for a bundle
    missing its manifest.

I considered raising the error directly from Bundle.js's constructor
instead of adding a plugin, but every message emitted there needs a
ruleId to satisfy the report JSON schema (test/fixtures/reportSchema.js,
enforced by test/specs/testAllPluginsMultipleBundlesReportSchema.js,
which scans every apiproxy fixture under test/fixtures/resources
against every registered plugin). A dedicated plugin gets a ruleId for
free via lintUtil.getRuleId()'s filename convention, matches how the
issue's "secondary enhancement" suggested a new rule for manifest/BN00x
checks, and keeps the check enable/disable-able like any other rule.

Verification

  • npm ci
  • npm run eslint — clean, no errors.
  • npm test (mocha, the same command CI runs via
    .github/workflows/run-tests.yml) — full suite passes:
    1330 passing, 0 failing (2 new specs added on top of the previous
    1326 passing / 2 that would otherwise fail against the new fixture
    without the ruleId fix).

Fixes #688.

@google-cla

google-cla Bot commented Aug 1, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

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.

ApigeeLint does not enforce existence of the manifest file

1 participant