-
Notifications
You must be signed in to change notification settings - Fork 25
Create Limits for the confidential-http capability #1810
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
✅ API Diff Results - No breaking changes |
…common into confidential-http-limits
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds per-workflow limit settings for the ConfidentialHTTP capability, mirroring the existing HTTPAction limits so that confidential HTTP calls are governed by the same configuration framework.
Changes:
- Extend the
Workflowsschema with a newConfidentialHTTPsection and correspondingconfidentialHttpstruct, defining call limit, connection timeout, and request/response size limits. - Update default configuration sources (
defaults.toml,defaults.json) to includePerWorkflow.ConfidentialHTTPdefaults aligned withHTTPAction. - Update tests and the README’s limits diagram to cover and document the new
ConfidentialHTTPsettings.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
pkg/settings/cresettings/settings.go |
Adds ConfidentialHTTP to the Workflows schema and defines the confidentialHttp struct and its default values, paralleling HTTPAction. |
pkg/settings/cresettings/settings_test.go |
Extends the schema unmarshal test to include ConfidentialHTTP config and asserts its default call and request size limits. |
pkg/settings/cresettings/defaults.toml |
Adds a [PerWorkflow.ConfidentialHTTP] block with default call, timeout, and size limits. |
pkg/settings/cresettings/defaults.json |
Adds a PerWorkflow.ConfidentialHTTP object mirroring the TOML defaults. |
pkg/settings/cresettings/README.md |
Updates the Mermaid limits diagram to include a PerWorkflow.ConfidentialHTTP subgraph and its bounds. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "CallLimit": "5", | ||
| "ConnectionTimeout": "10s", | ||
| "RequestSizeLimit": "10kb", | ||
| "ResponseSizeLimit": "100kb" |
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The indentation on this JSON line is inconsistent with the surrounding block (it appears to use different spacing than the other key/value lines), which makes the embedded defaults harder to read and maintain. Please align the indentation with the other fields in this object for consistency.
pkg/settings/cresettings/settings.go
Outdated
| RequestSizeLimit Setting[config.Size] | ||
| ResponseSizeLimit Setting[config.Size] | ||
| } | ||
| type confidentialHttp struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| type confidentialHttp struct { | |
| type confidentialHTTP struct { |
pkg/settings/cresettings/settings.go
Outdated
| ChainRead chainRead | ||
| Consensus consensus | ||
| HTTPAction httpAction | ||
| ConfidentialHTTP confidentialHttp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to swap the words so it groups with HTTP? 🤷
| ConfidentialHTTP confidentialHttp | |
| HTTPConfidential confidentialHttp |
Or would another sub-type like HTTPAction.Confidential make sense?
I don't feel strongly about either. Just exploring options.
The limits just mirror what's there for http-actions capability.