Skip to content

fix: bind after service create, and keep manifest fields on deploy - #6004

Merged
norman-abramovitz merged 6 commits into
cloudfoundry:developfrom
nabramovitz:norm/fix/push-manifest-fields
Oct 14, 2026
Merged

norman-abramovitz merged 6 commits into
cloudfoundry:developfrom
nabramovitz:norm/fix/push-manifest-fields

Conversation

@nabramovitz

Copy link
Copy Markdown
Contributor

Two bug fixes found while looking at async service creation and label support (#4089).

Bind after create (#6001)

The create-service-instance wizard reads the new instance's guid from the finished CF job's links, under
links.service_instance. Cloud Controller names that link after the job's resource type, which is
service_instances for create and update jobs (only the delete job uses the singular). The guid was never found, so
the bind was skipped without an error. Checked against a live CF (API 3.226.0): the create job returned
links.service_instances.

The frontend now reads the plural key. The spec and translator test fixtures used the singular key, which is why the
tests passed; they now use the shape CF sends.

Manifest fields on deploy (#6003)

fetchManifest added STRATOS_PROJECT by loading the manifest into RawManifestApplication (24 fields) and
writing that back, dropping everything else: metadata, processes, sidecars, readiness-health-check-*,
log-rate-limit-per-second, the top-level version. It now edits the YAML document and sets only the env entry. An
application without its own env block gets one from the decoded struct, so env inherited through a YAML merge key
is kept.

Tests

  • TestFetchManifest_KeepsFieldsItDoesNotModel failed on all six fields before the fix.
  • TestFetchManifest_AddsEnvVarAlongsideExistingEnv covers an existing env block and a merge-key app; it fails if the
    merge-key branch builds env from scratch.
  • The bind spec fails with the plural fixture before the fix.
  • make check gate passes.

Changelog fragment: changelog.d/0007-service-bind-and-manifest-fields.md.

AI disclosure (RFC-0047): prepared with AI coding assistance; reviewed by the author.

Cloud Controller names a job's resource link after its resource type,
so create and update jobs return links.service_instances; only the
delete job uses the singular. The wizard read service_instance, never
found the new guid and skipped the bind without an error. The spec
and translator fixtures used the singular key, so the tests passed.

Fixes cloudfoundry#6001
fetchManifest added STRATOS_PROJECT by loading the manifest into
RawManifestApplication and writing that back, which dropped every
field the struct does not model: metadata, processes, sidecars, the
newer health check and log rate keys, the top-level version. Edit the
YAML document instead and set only the env entry. An app without its
own env block gets one from the decoded struct, so env it inherits
through a merge key is kept.

Fixes cloudfoundry#6003
The guide asked for 48-character subjects without a type prefix,
while CONTRIBUTING.md and the history use type(scope): summary,
which release tooling reads. Point at CONTRIBUTING.md, aim for 50
characters and cap subjects at 72, where GitHub truncates them.

@norman-abramovitz norman-abramovitz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM - Need more tests

@norman-abramovitz norman-abramovitz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM for the updated files

@nabramovitz

Copy link
Copy Markdown
Contributor Author

#6005 adds tests that answer the wizard's writes with recorded CF responses, with the #6001 bind cases marked as expected failures. Merge #6005 first; this branch then merges develop and drops those markers, and the bind cases pass on both resolve paths, for sync and async brokers.

With the plural job link read, the wizard binds the chosen app for a
synchronous and an asynchronous broker on the fast and the polled
path, so the cloudfoundry#6001 expected-failure markers from cloudfoundry#6005 come off, in
the unit spec and the e2e spec. The service write tests fragment no
longer lists bind after create as a known gap.

@norman-abramovitz norman-abramovitz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM Approving additional changes to the service broker unit tests.

@norman-abramovitz
norman-abramovitz merged commit 8d5cd85 into cloudfoundry:develop Oct 14, 2026
24 checks passed
@nabramovitz
nabramovitz deleted the norm/fix/push-manifest-fields branch October 14, 2026 07:24
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.

2 participants