Skip to content
This repository was archived by the owner on Nov 28, 2025. It is now read-only.

Conversation

@shults
Copy link

@shults shults commented Oct 13, 2025

User description

Contributor checklist

  • Reviewed PR Code suggestions and updated accordingly
  • Tyklings: Labled the PR with the relevant releases
  • Tyklings: Added Jira DX PR ticket to the subject

New Contributors



PR Type

Documentation


Description

  • Add OPA helpers for Tyk OAS/streams/classic

  • Gate patch rules with is_tyk_classic

  • Document maintenance warning for exclusions

  • Sync shared OPA rules snippet


Diagram Walkthrough

flowchart LR
  OPA["OPA policy snippets"] -- "add helpers" --> Helpers["is_tyk_oas / is_tyk_streams / is_tyk_classic"]
  Helpers -- "used to gate" --> PatchRules["patch_request examples"]
  Notes["Maintenance warnings"] -- "exclusion-based risks" --> isClassic["is_tyk_classic"]
  Shared["shared/opa-rules.md"] -- "synchronize rules" --> Helpers
Loading

File Walkthrough

Relevant files
Documentation
dashboard-configuration.md
OPA helpers and guards added to dashboard config docs       

tyk-docs/content/api-management/dashboard-configuration.md

  • Add is_tyk_oas, is_tyk_streams, is_tyk_classic rules.
  • Gate patch_request examples with is_tyk_classic.
  • Insert detailed maintenance warnings for exclusions.
  • Duplicate rules across relevant sections for consistency.
+85/-0   
opa-rules.md
Shared OPA rules for API type detection                                   

tyk-docs/content/shared/opa-rules.md

  • Add shared OPA rules for API-type detection.
  • Include maintenance warning for exclusion approach.
  • Align shared snippet with dashboard docs examples.
+41/-0   

@github-actions
Copy link
Contributor

⚠️ Deploy preview for PR #7054 did not become live after 3 attempts.
Please check Netlify or try manually: Preview URL

@github-actions
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Formatting/Indentation

The Rego snippets show inconsistent indentation (tabs vs spaces) and mixed inline alignment for rule bodies which may confuse readers or break copy-paste usability. Ensure consistent formatting across examples.

# Rule is_tyk_oas check if endpoint belongs to oas api.
default is_tyk_oas = false
	is_tyk_oas {
	startswith(input.request.path, "/api/apis/oas")
}

# Rule is_tyk_streams check if endpoint belongs to streams api.
default is_tyk_streams = false
	is_tyk_streams {
	startswith(input.request.path, "/api/apis/streams")
}

# Rule is_tyk_classic check if endpoint belongs to classic api.
# WARNING: CRITICAL MAINTENANCE REQUIREMENT
# The is_tyk_classic rule below is defined by exclusion (not is_tyk_oas and not is_tyk_streams).
# This approach is brittle and not forward-compatible.
#
# IMPORTANT: Whenever a new API type is introduced under the "/api/apis/" path,
# this rule MUST be explicitly updated to exclude the new type.
# Failure to do so will result in the new API type being incorrectly classified as "classic",
# which may lead to incorrect authorization policies being applied.
#
# Example: If a new API type "graphql" is added with path "/api/apis/graphql",
# this rule must be updated to:
#
# is_tyk_classic {
#     not is_tyk_oas
#     not is_tyk_streams
#     not is_tyk_graphql  # Add exclusion for the new API type
#     startswith(input.request.path, "/api/apis")
# }
#
# Consider refactoring this rule to use positive matching instead of exclusion
# if a specific pattern for classic APIs can be identified.
default is_tyk_classic = false
is_tyk_classic {
	not is_tyk_oas
	not is_tyk_streams
	startswith(input.request.path, "/api/apis")
}
Rule Robustness

The is_tyk_classic rule relies on exclusion. While warnings are documented, consider providing a positive-match alternative example to reduce operational risk and reader error.

# Rule is_tyk_classic check if endpoint belongs to classic api.
# WARNING: CRITICAL MAINTENANCE REQUIREMENT
# The is_tyk_classic rule below is defined by exclusion (not is_tyk_oas and not is_tyk_streams).
# This approach is brittle and not forward-compatible.
#
# IMPORTANT: Whenever a new API type is introduced under the "/api/apis/" path,
# this rule MUST be explicitly updated to exclude the new type.
# Failure to do so will result in the new API type being incorrectly classified as "classic",
# which may lead to incorrect authorization policies being applied.
#
# Example: If a new API type "graphql" is added with path "/api/apis/graphql",
# this rule must be updated to:
#
# is_tyk_classic {
#     not is_tyk_oas
#     not is_tyk_streams
#     not is_tyk_graphql  # Add exclusion for the new API type
#     startswith(input.request.path, "/api/apis")
# }
#
# Consider refactoring this rule to use positive matching instead of exclusion
# if a specific pattern for classic APIs can be identified.
default is_tyk_classic = false
is_tyk_classic {
	not is_tyk_oas
	not is_tyk_streams
	startswith(input.request.path, "/api/apis")
}
Duplication Divergence

The OPA helper rules are duplicated across this file and dashboard-configuration.md; ensure they remain synchronized or reference the shared snippet to avoid drift.

# Rule is_tyk_oas check if endpoint belongs to oas api.
default is_tyk_oas = false
is_tyk_oas {
	startswith(input.request.path, "/api/apis/oas")
}

# Rule is_tyk_streams check if endpoint belongs to streams api.
default is_tyk_streams = false
is_tyk_streams {
	startswith(input.request.path, "/api/apis/streams")
}

# Rule is_tyk_classic check if endpoint belongs to classic api.
# WARNING: CRITICAL MAINTENANCE REQUIREMENT
# The is_tyk_classic rule below is defined by exclusion (not is_tyk_oas and not is_tyk_streams).
# This approach is brittle and not forward-compatible.
#
# IMPORTANT: Whenever a new API type is introduced under the "/api/apis/" path,
# this rule MUST be explicitly updated to exclude the new type.
# Failure to do so will result in the new API type being incorrectly classified as "classic",
# which may lead to incorrect authorization policies being applied.
#
# Example: If a new API type "graphql" is added with path "/api/apis/graphql",
# this rule must be updated to:
#
# is_tyk_classic {
#     not is_tyk_oas
#     not is_tyk_streams
#     not is_tyk_graphql  # Add exclusion for the new API type
#     startswith(input.request.path, "/api/apis")
# }
#
# Consider refactoring this rule to use positive matching instead of exclusion
# if a specific pattern for classic APIs can be identified.
default is_tyk_classic = false
is_tyk_classic {
	not is_tyk_oas
	not is_tyk_streams
	startswith(input.request.path, "/api/apis")
}

@github-actions
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Correct invalid rule indentation

Fix indentation of rule headers; the extra leading tab before is_tyk_oas and
is_tyk_streams can cause parse or policy load issues. Align rule names to column 0.

tyk-docs/content/api-management/dashboard-configuration.md [5267-5277]

 default is_tyk_oas = false
-	is_tyk_oas {
+is_tyk_oas {
 	startswith(input.request.path, "/api/apis/oas")
 }
 
 default is_tyk_streams = false
-	is_tyk_streams {
+is_tyk_streams {
 	startswith(input.request.path, "/api/apis/streams")
 }
Suggestion importance[1-10]: 7

__

Why: The indentation in the PR shows rule headers with leading tabs, which is improper in Rego examples and could confuse readers or break copy-paste; aligning to column 0 is accurate and improves correctness.

Medium
Prevent future misclassification risk

The exclusion-based is_tyk_classic risks misclassification when new API types are
added. Replace it with positive matching on known classic paths to prevent future
breakage.

tyk-docs/content/shared/opa-rules.md [224-263]

 default is_tyk_oas = false
 is_tyk_oas {
 	startswith(input.request.path, "/api/apis/oas")
 }
 
 default is_tyk_streams = false
 is_tyk_streams {
 	startswith(input.request.path, "/api/apis/streams")
 }
 
+# Positive match for classic APIs (adjust as needed to true classic prefixes)
 default is_tyk_classic = false
 is_tyk_classic {
-	not is_tyk_oas
-	not is_tyk_streams
-	startswith(input.request.path, "/api/apis")
+	startswith(input.request.path, "/api/apis")    # base
+	not startswith(input.request.path, "/api/apis/oas")
+	not startswith(input.request.path, "/api/apis/streams")
 }
Suggestion importance[1-10]: 5

__

Why: The concern about exclusion-based is_tyk_classic is valid and relevant, but the proposed code still relies on exclusions and does not provide definitive positive matching; impact is moderate and somewhat speculative without defined classic prefixes.

Low
Avoid nil field access

Guard against missing api_definition or name to avoid runtime errors on non-classic
payloads. Add has checks before calling contains.

tyk-docs/content/api-management/dashboard-configuration.md [4740-4746]

 patch_request[x] {
    request_permission[_] == "apis"
    request_intent == "write"
    is_tyk_classic
+   input.request.body.api_definition.name
    contains(input.request.body.api_definition.name, "#external")
 
    x := {"api_definition": {"proxy": {"transport": {"proxy_url": "http://company-proxy:8080"}}}}
 }
Suggestion importance[1-10]: 3

__

Why: The intent to guard against missing fields is reasonable, but the proposed input.request.body.api_definition.name line is not a proper has-check in Rego and doesn't prevent errors; correctness is limited, so impact is low.

Low

@netlify
Copy link

netlify bot commented Oct 13, 2025

PS. Add to the end of url /docs/nightly

Name Link
🔨 Latest commit 10e76fd
🔍 Latest deploy log https://app.netlify.com/projects/tyk-docs/deploys/68ecf764a4fe900008b27738
😎 Deploy Preview https://deploy-preview-7054--tyk-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@netlify
Copy link

netlify bot commented Oct 13, 2025

PS. Add to the end of url /docs/nightly

Name Link
🔨 Latest commit 9a1b2b0
🔍 Latest deploy log https://app.netlify.com/projects/tyk-docs/deploys/690ca8894b77fa00083c80b3
😎 Deploy Preview https://deploy-preview-7054--tyk-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link
Contributor

@andyo-tyk andyo-tyk left a comment

Choose a reason for hiding this comment

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

I've made suggested changes to the shared opa_rules.md file.

@sharadregoti Is this automatically embedded in the dashboard_configuration.md file?

If not, then please update that file to match.

not input.user.group_name == "TeamA-Admin"
}
# Rule is_tyk_oas check if endpoint belongs to oas api.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Rule is_tyk_oas check if endpoint belongs to oas api.
# Rule is_tyk_oas identifies if the API is Tyk OAS by checking the endpoint used for the operation.

startswith(input.request.path, "/api/apis/oas")
}
# Rule is_tyk_streams check if endpoint belongs to streams api.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Rule is_tyk_streams check if endpoint belongs to streams api.
# Rule is_tyk_streams identifies if the API is Tyk Streams by checking the endpoint used for the operation.

startswith(input.request.path, "/api/apis/streams")
}
# Rule is_tyk_classic check if endpoint belongs to classic api.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Rule is_tyk_classic check if endpoint belongs to classic api.
# Rule is_tyk_classic identifies if the API is Tyk Classic by exclusion (i.e. not Tyk OAS and not Tyk Streams).

#
# IMPORTANT: Whenever a new API type is introduced under the "/api/apis/" path,
# this rule MUST be explicitly updated to exclude the new type.
# Failure to do so will result in the new API type being incorrectly classified as "classic",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Failure to do so will result in the new API type being incorrectly classified as "classic",
# Failure to do so will result in the new API type being incorrectly classified as "Tyk Classic",

# }
#
# Consider refactoring this rule to use positive matching instead of exclusion
# if a specific pattern for classic APIs can be identified.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# if a specific pattern for classic APIs can be identified.

default is_tyk_classic = false
is_tyk_classic {
not is_tyk_oas
not is_tyk_streams
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
not is_tyk_streams

is_tyk_classic {
not is_tyk_oas
not is_tyk_streams
startswith(input.request.path, "/api/apis")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
startswith(input.request.path, "/api/apis")

not is_tyk_oas
not is_tyk_streams
startswith(input.request.path, "/api/apis")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
}

# Failure to do so will result in the new API type being incorrectly classified as "classic",
# which may lead to incorrect authorization policies being applied.
#
# Example: If a new API type "graphql" is added with path "/api/apis/graphql",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Example: If a new API type "graphql" is added with path "/api/apis/graphql",
# Example: If a new API type "graphql" is added, you should add an identifier

# which may lead to incorrect authorization policies being applied.
#
# Example: If a new API type "graphql" is added with path "/api/apis/graphql",
# this rule must be updated to:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# this rule must be updated to:
# helper (e.g. is_tyk_graphql) and this rule must be updated to:

@shults shults requested a review from andyo-tyk October 14, 2025 07:39
@shults shults closed this Oct 20, 2025
@shults shults reopened this Oct 27, 2025
@shults shults force-pushed the TT-10073-support-tyk-oas-with-open-policy-agent branch from a90edf0 to b8708b5 Compare October 27, 2025 14:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants