You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This adds a BillingFeature kind to frontier reconcile and frontier export. Gitops can now manage each billing feature's title and metadata, for example a markdown tooltip.
apiVersion: v1kind: BillingFeaturespec:
- name: aoi_size_for_taskingtitle: "Max size for each Tasking AOI: 2,500 km²"metadata:
info: "Imagery is charged separately. Contact [support](mailto:support@example.com)."
Why
Today the BillingProduct kind lists a product's features by name only. A feature's title and metadata could only be set by hand in the DB. The server also ignores title and metadata for features that already exist when a product is updated.
How it works
It follows RFC 0001.
Identity: the feature name, matched exactly as the server stores it, including case and spaces. This is the same as the API, where Foo and foo are two features.
Field rule: a title left out becomes "". Metadata left out becomes {}.
Features are objects: an in-scope server feature missing from the file fails the plan. delete: true is rejected, because there is no API to delete a feature.
Name checks: exact duplicate names and blank names fail the plan. A name that differs from a server feature only by case is created as a new feature, like the API does.
Short names: server features with names shorter than 3 characters are out of scope, the same as BillingProduct. They are not required in the file and not exported. A file entry with the exact name of one fails the plan.
Product links stay with BillingProduct:
An update sends back the feature's current product links, so a feature is never unlinked.
A create sends no links.
Order: in one file, BillingFeature must come before BillingProduct.
Validation: every planned request is checked against the API's own rules before anything is applied.
Export: writes name, title and metadata, sorted by name. Reconciling an export plans no changes.
Commits
Spec and diff
Reconciler
Export
Kind order rule
CLI registration
Docs, covering the reconcile guide, the CLI reference and an addendum to RFC 0001. The published RFC itself is not changed.
Tests
Unit tests:
the diff cases
product links sent back on update
a dry run makes no writes
validation
the export round trip, including nested metadata
short names out of scope, matched by exact name
a server feature with unset metadata
kind order, checking that no product is written before the order error
A local sandbox run:
apply, then a rerun with no changes
ListFeatures and ListPlans show the new title and metadata
product links unchanged
a reset, a missing feature, and the wrong kind order all behave as expected
Related
This needs the UpdateFeature change in #2045. Without it, the server ignores an empty title or empty metadata, so a reset would plan but not apply. The two PRs can be reviewed and merged in either order. The CLI and the server are expected to run the same version.
The product link bug found during review is tracked in #2047.
Added support for exporting and reconciling billing features, including creating missing features and updating titles and metadata.
Reconciliation validates feature names and metadata, supports dry runs, and preserves existing product links when updating features.
Billing product documents must appear before billing feature documents. Reconciliation reports errors when features cannot be managed or deleted.
Documentation
Updated CLI and reconciliation guides with billing feature support, ordering rules, and deletion limitations, and added a billing feature RFC addendum.
Adds billing-feature reconciliation with validation, change detection, API operations, and export. Registers the reconciler and requires billing-feature documents to precede billing-product documents. Updates the CLI reference, reconciliation guide, and RFC addendum.
Changes: Billing feature reconciliation
Layer / File(s)
Summary
Feature specification and diff internal/reconcile/billingfeature.go, internal/reconcile/billingfeature_test.go
Adds billing-feature spec validation and change detection for names, titles, and metadata. The diff reports unlisted server features and sorts additions and updates by name. Tests cover validation, change detection, and operation ordering.
Reconciler API and operations internal/reconcile/billingfeature_reconciler.go, internal/reconcile/billingfeature_reconciler_test.go
Adds API-backed listing, scope filtering, reconciliation, dry runs, create and update requests, and export. Tests cover API errors, partial reports, request fields, and export behavior.
Registry and document ordering cmd/reconcile.go, internal/reconcile/reconcile.go, internal/reconcile/reconcile_test.go
Registers the billing-feature reconciler and requires billing-feature documents to precede billing-product documents. A test checks ordering behavior.
raystack/frontier#2045: Adds the UpdateFeature field replacement behavior used by billing-feature reconciliation to apply desired title and metadata values.
Suggested reviewers:rohanchkrabrty
Merge Risk: Moderate · up to 3cdc0
Applying BillingFeature files can overwrite product links that changed concurrently. Clearing a title or metadata only works with a server that includes #2045. The docs do not yet warn about product creation overwriting a feature's links. The feature is usable for the normal flow, but these points should be resolved or explicitly accepted before merge.
Pre-merge checks: 2
Check name
Status
Explanation
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Nitpick comments: 2
internal/reconcile/reconcile_test.go: 1
169-172: Trivial · Maintainability & Code Quality · Low value
Assert that no product write occurs before the ordering error.
Use a valid, nonempty product spec and assert that productAPI.created remains empty. The empty spec produces no product writes, so the test can pass if product reconciliation runs before the ordering check.
83-85: Trivial · Data Integrity & Integration · Heavy lift
Test reset persistence through the server.
This test checks only the request sent to a fake. The current server retains the old title and metadata when the update sends empty values, so the test cannot detect the failed reset. Add an integration test that reloads both fields after applying the reset and verifies that a second reconciliation plans no changes.
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review.
The reason will be displayed to describe this comment to others. Learn more.
Major · Data Integrity & Integration · Heavy lift
Supported by static analysis
Script executed:
sed -n '145,200p' internal/reconcile/billingfeature_reconciler.go
sed -n '225,260p' internal/api/v1beta1connect/billing_product.go
sed -n '570,605p' billing/product/service.go
rg -n 'UpsertFeature\(' billing internal/api
sed -n '260,285p' docs/content/docs/reconcile.mdx
Repository: raystack/frontier
Length of output: 8321
Script executed:
forspecin'billing/product/service.go:105:135''billing/product/service.go:600:645''billing/plan/service.go:145:172''billing/plan/service.go:235:285''internal/reconcile/billingfeature_reconciler.go:1:110''internal/reconcile/billingfeature.go:1:160''cmd/reconcile.go:1:110';do IFS=: read -r f start end <<<"$spec";echo"--- $f:$start-$end"; nl -ba "$f"| sed -n "${start},${end}p";done
rg -n -F -e 'NewBillingFeatureReconciler' -e 'NewFrontierServiceClient' -e 'frontierURL' -e 'frontier_url' -- cmd internal/reconcile
Repository: raystack/frontier
Length of output: 23756
Script executed:
forspecin'internal/reconcile/billingfeature_reconciler.go:109:164''internal/reconcile/billingfeature_reconciler_test.go:14:98''internal/store/postgres/billing_feature_repository.go:178:220''pkg/metadata/metadata.go:15:35''cmd/client.go:1:75';do IFS=: read -r f start end <<<"$spec";echo"--- $f:$start-$end"; nl -ba "$f"| sed -n "${start},${end}p";done
rg -n -F -e 'Host ' -e '"host"' -e 'FRONTIER_HOST' -- cmd/root.go cmd/config.go cmd/client.go
Repository: raystack/frontier
Length of output: 11758
Support and check empty field replacement.
When a title or metadata is omitted, the current server retains the existing value. Reconcile reports success, but later runs plan the same update.
Add replacement support to UpdateFeature without changing partial updates for other UpsertFeature callers. Check the returned feature against the desired values and report an error if it did not converge. Document that resets require a server with replacement support, and test both successful resets and nonconverging responses.
The reason will be displayed to describe this comment to others. Learn more.
Analysis chain
Script executed:
#!/bin/bashset -o pipefail
echo'--- PR metadata ---'
gh pr view 2045 --repo raystack/frontier --json title,state,mergedAt,baseRefName,headRefName,body,files
echo'--- Relevant diff excerpts ---'
gh pr diff 2045 --repo raystack/frontier | python3 -c 'import syss = sys.stdin.read()for part in s.split("diff --git "): if not part.strip(): continue lines = part.splitlines() if any(k in part for k in ("UpdateFeature", "UpsertFeature", "ReplaceFeature", "metadata", "Metadata")): print("diff --git " + lines[0]) for i, line in enumerate(lines): if any(k in line for k in ("UpdateFeature", "UpsertFeature", "ReplaceFeature", "metadata", "Metadata", "ProductIds", "ProductIDs")): print("\n".join(lines[max(1, i-3):min(len(lines), i+5)])) print("---")'| head -c 30000
Length of output: 33942
@rohilsurana Understood. I won’t pursue a CLI response check for a version mismatch.
#2045 is still open and contains the server-side replacement change. Before release, please verify that it is included and that a reset followed by a rerun plans no changes.
The reason will be displayed to describe this comment to others. Learn more.
Agreed. #2045 must ship before or with this PR. Reset was tested end to end in a local sandbox with both changes: clearing a title and metadata was applied, and a rerun planned no changes.
nl -ba internal/reconcile/billingproduct.go | sed -n '198,345p'
nl -ba billing/product/service.go | sed -n '230,290p;670,710p'
nl -ba billing/entitlement/service.go | sed -n '1,180p'
nl -ba docs/content/docs/reconcile.mdx | sed -n '245,330p'
nl -ba internal/reconcile/billingproduct_reconciler_test.go | sed -n '275,375p'
git diff f38ac119be14d4fda11f874ecfad9b2c4d9b65e8 22fa519476ecf9d61ffe6d9539b00e4bc72d5b20 -- internal/reconcile/billingproduct.go internal/reconcile/billingproduct_reconciler.go billing/product/service.go internal/store/postgres/billing_feature_repository.go
Repository: raystack/frontier
Length of output: 24592
Preserve existing feature links when creating a product.
If a new product references a feature already linked to another product, creation replaces the feature’s product IDs with the new ID. The earlier product can lose the feature and its subscribers can lose entitlement. Merge the new product ID with the existing links on the create path.
Files skipped from review as they are similar to previous changes: 1
docs/content/docs/reconcile.mdx
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.
The reason will be displayed to describe this comment to others. Learn more.
Minor · Functional Correctness · Quick win
Exclude out-of-scope features from the full-list requirement.
Features with names shorter than three characters are omitted from export. If a user follows this requirement and adds one to the file, reconciliation fails. State that only in-scope features must be listed, and that short-name entries are rejected.
The reason will be displayed to describe this comment to others. Learn more.
This is on purpose for now. The API treats Foo and foo as two features: name is a case-sensitive unique column (migrations/20240106162115_refactor_features.up.sql:20) and lookup is an exact match (billing_feature_repository.go:125). Reconcile matches the API, so two new names that differ only by case are both created. We can tighten this later if the API starts treating names case-insensitively.
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.
The reason will be displayed to describe this comment to others. Learn more.
Minor · Data Integrity & Integration · Quick win
Document numeric precision loss.
Metadata numbers use float64. For example, 9007199254740993 becomes 9007199254740992, and the diff can treat them as equal. (raw.githubusercontent.com) Warn users to quote values that require exact precision.
The reason will be displayed to describe this comment to others. Learn more.
Major · Data Integrity & Integration · Quick win
Integrate the server reset behavior before enabling empty resets.
When an existing feature has a title or metadata, omitting those fields now plans an update. The reconciler sends the empty values through UpdateFeature. However, billing/product/service.go still uses nonempty guards in UpsertFeature, so the server retains the stored values. Apply reports success, and the next reconcile plans the same update again. (raw.githubusercontent.com)
Integrate the stated #2045 dependency before releasing this behavior. Verify that applying an empty title and metadata produces a no-change subsequent plan.
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review.
The reason will be displayed to describe this comment to others. Learn more.
Major · Data Integrity & Integration · Quick win
Document the product-link overwrite.
Product creation does not merge existing product links. If a new product uses an existing feature, UpsertFeature replaces that feature’s links with the new product’s link. Customers on the other products can lose the feature. (raw.githubusercontent.com)
Until issue #2047 is fixed, correct this claim and add a warning to the reconciliation guide.
The reason will be displayed to describe this comment to others. Learn more.
Major · Data Integrity & Integration · Heavy lift
Prevent feature updates from overwriting newer product links.
If another caller changes a feature’s product links after fetchCurrent runs, Line 183 sends the old links with the title or metadata update. The server replaces the stored links with that list. A title edit can therefore undo a product-link change and affect entitlements. Update only the fields managed by BillingFeature, or require a version check before replacing links. (raw.githubusercontent.com)
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
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
What
This adds a
BillingFeaturekind tofrontier reconcileandfrontier export. Gitops can now manage each billing feature'stitleandmetadata, for example a markdown tooltip.Why
Today the
BillingProductkind lists a product's features by name only. A feature's title and metadata could only be set by hand in the DB. The server also ignores title and metadata for features that already exist when a product is updated.How it works
It follows RFC 0001.
Fooandfooare two features."". Metadata left out becomes{}.delete: trueis rejected, because there is no API to delete a feature.BillingProduct. They are not required in the file and not exported. A file entry with the exact name of one fails the plan.BillingProduct:BillingFeaturemust come beforeBillingProduct.Commits
Tests
ListFeaturesandListPlansshow the new title and metadataRelated
This needs the
UpdateFeaturechange in #2045. Without it, the server ignores an empty title or empty metadata, so a reset would plan but not apply. The two PRs can be reviewed and merged in either order. The CLI and the server are expected to run the same version.The product link bug found during review is tracked in #2047.