Honor filter for service-level policy; document policies key and sub-filters#221
Merged
Conversation
… sub-filters The extractServicePolicy function was called without a filter argument and never consulted shouldIncludeResource, so a filter of policies: [] still emitted apim-artifacts/policy.xml. Thread the filter through and add the guard so an empty list truly excludes the singleton. Documentation gaps found in the same investigation: - docs/commands/extract.md was missing the policies filter key and did not describe the nested apiSubFilters / workspaceSubFilters syntax accepted by parseFilterArrayWithNested. - docs/reference/dependency-graph.md claimed extract does not auto-include dependencies, which contradicted the transitive resolver. Changes: - extractServicePolicy now takes filter and returns early via shouldIncludeResource, matching every other extract-* helper. - Add three unit tests covering policies: [], omitted, and ["policy"]. - docs/commands/extract.md: add policies row plus apiSubFilters and workspaceSubFilters sections with nested YAML examples. - docs/reference/dependency-graph.md: rewrite the Filtering Considerations paragraph to separate publish-time vs extract-time behavior. Verified with 45 live filter scenarios against a source APIM instance; the previously leaked policy.xml no longer appears in the exclude-all run. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
petehauge
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes apiops extract --filter so the service-level policy (the singleton apim-artifacts/policy.xml) correctly respects the policies filter key, and updates documentation to reflect the full set of supported filter keys plus nested sub-filter syntax.
Changes:
- Threads
filterintoextractServicePolicyand guards extraction withshouldIncludeResource(...)sopolicies: []excludes the singleton policy. - Adds unit tests covering the three intended filter states for service policy:
policies: [],policiesomitted, andpolicies: ['policy']. - Updates docs to include the missing
policieskey, document API/workspace nested sub-filters, and clarify extract-time vs publish-time dependency behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/services/extract-service.ts |
Ensures service-level policy extraction is filtered via shouldIncludeResource and threads filter through the helper. |
tests/unit/services/extract-service.test.ts |
Adds regression coverage for service policy filter inclusion/exclusion semantics. |
docs/commands/extract.md |
Documents the policies filter key and nested apiSubFilters / workspaceSubFilters YAML forms. |
docs/reference/dependency-graph.md |
Clarifies publish-time “no auto-include” vs extract-time limited transitive auto-follow behavior. |
Issue #218 reported that a filter with only �pis: [my-api] set still extracted every backend, named value, product, etc. That is the documented behavior — each top-level key is independent, and omitted keys default to "include all" — but the docs and the init-generated template did not make this clear enough, and the filtering guide's Quick Start actively claimed the opposite. Changes: - docs/guides/filtering-resources.md: fix misleading Quick Start claim and add a "Common Pitfall: Narrowing to Just One API" section with a complete worked example that sets every other type to []. - docs/commands/extract.md: add a callout under the value-semantics list pointing at the new pitfall section. - src/templates/configs/filter-config.yaml: prepend a "HOW FILTERING WORKS" block so the first thing a user sees in their init-generated configuration.extractor.yaml explains the three states and shows the narrow-to-one-API pattern. - Regenerated src/templates/generated/embedded-markdown.ts (prelint). Refs #218 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
EMaher
previously approved these changes
Jul 2, 2026
Per PR review from @EMaher: - docs/commands/extract.md: promote the plain blockquote to a GitHub markdown alert (> [!TIP]) so it renders as a visual callout. - docs/guides/filtering-resources.md: rename "Common Pitfall: Narrowing to Just One API" to "How To: Extract Just One API" — the section is a how-to, not a warning. - src/templates/configs/filter-config.yaml: soften the "Common pitfall" wording in the init-generated template to "Tip:"; drop the negative framing from the leading comment block. Also updated the extract.md anchor and the Quick Start callout in the filtering guide to match the renamed section. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
EMaher
approved these changes
Jul 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Systematic testing of
apiops extract --filter(45 scenarios against a live source APIM) surfaced one code bug and two documentation gaps. This PR fixes all three.Bug: service policy leaked past filter
extractServicePolicywas called without a filter argument and never consultedshouldIncludeResource, so a filter ofpolicies: []still emittedapim-artifacts/policy.xml. Every other extract-* helper honors the filter; this one did not.Fix: thread
filterintoextractServicePolicyand guard withshouldIncludeResource(descriptor, filter). BecauseServicePolicy.nameParts = [],getSingletonFilterNamereturns"policy", so:policies: []→ excluded ✅policieskey omitted → included (matches "field absent means include all") ✅policies: ['policy']→ included ✅Docs: missing
policieskey + sub-filter syntaxdocs/commands/extract.mdlisted 16 filter keys; the loader accepts 17 (policieswas missing). It also had no reference for the nestedapiSubFilters/workspaceSubFilterssyntax thatparseFilterArrayWithNestedaccepts.Docs: contradiction re auto-inclusion
docs/reference/dependency-graph.mdsaid extract "does not auto-include dependencies", which contradicts the transitive resolver behavior. Rewrote to separate publish-time vs extract-time behavior.Changes
src/services/extract-service.ts— importshouldIncludeResource; addfilterparameter toextractServicePolicy; guard at top of functiontests/unit/services/extract-service.test.ts— 3 new tests covering the three filter states ([], omitted,['policy'])docs/commands/extract.md— addpoliciesrow to filter table; add "API sub-filters" and "Workspace sub-filters" sections with nested YAML examplesdocs/reference/dependency-graph.md— rewrite "Filtering Considerations" paragraphVerification
npm test— 1069/1069 passnpm run lint— cleannpm run build— cleanservicePolicyLeakedcount went from 1 → 0 in the exclude-all runRelated Issue(s)
None filed — issues surfaced during investigation and fixed in the same branch.