Skip to content

Conversation

@kathap
Copy link
Contributor

@kathap kathap commented Sep 26, 2025

Stop building storage-cli JSON inside CC; instead consume files rendered by capi job templates. StorageCliClient now selects the JSON file by resource_type.

Thanks for contributing to cloud_controller_ng. To speed up the process of reviewing your pull request please provide us with:

  • A short explanation of the proposed change:
    Adopt storage_cli_client to read json configs rendered by capi. Detect correct json file by resource_type.

  • An explanation of the use cases your change solves
    Switch responsibility of JSON generation to capi-release so CC only reads well-formed per-resource configs. This removes duplication of storage cli config file creation.

  • Links to any other associated PRs
    Use single blobstore_configs.json + pre-start to write config files capi-release#580

  • I have reviewed the contributing guide

  • I have viewed, signed, and submitted the Contributor License Agreement

  • I have made this pull request to the main branch

  • I have run all the unit tests using bundle exec rake

  • I have run CF Acceptance Tests

Stop building storage-cli JSON inside CC; instead consume files
  rendered by capi job templates. StorageCliClient now selects the JSON
  file by `resource_type`.
@kathap kathap marked this pull request as draft September 26, 2025 14:07
@kathap kathap force-pushed the azure-storage-cli-integration branch 2 times, most recently from 05de380 to dbf843c Compare September 30, 2025 13:41
@kathap kathap force-pushed the azure-storage-cli-integration branch from dbf843c to 8c8c630 Compare September 30, 2025 14:03
@kathap kathap marked this pull request as ready for review October 1, 2025 13:10
@kathap kathap marked this pull request as draft October 8, 2025 08:06
@kathap kathap force-pushed the azure-storage-cli-integration branch 2 times, most recently from e763ccc to 4d069a4 Compare October 14, 2025 15:19
@kathap kathap force-pushed the azure-storage-cli-integration branch from 4d069a4 to 4c7d0cc Compare October 14, 2025 15:23
@kathap kathap marked this pull request as ready for review October 15, 2025 07:09
Copy link
Contributor

@jochenehret jochenehret left a comment

Choose a reason for hiding this comment

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

Looks good, just some minor findings.

raise BlobstoreError.new("Unknown resource_type: #{resource_type}")
end

path = VCAP::CloudController::Config.config.get(key)
Copy link
Contributor

Choose a reason for hiding this comment

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

What if config.get(key) returns nil? Maybe include key in the error message?

Copy link
Contributor Author

@kathap kathap Oct 21, 2025

Choose a reason for hiding this comment

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

If resource_type does not match any of the specified cases line 50-54, the else branch will raise a BlobstoreError before key is assigned, so execution will not reach line 59 with a nil value for key.

json
end

def config_path_for!(resource_type)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why an exclamation mark in the method name? It doesn't modify the passed object. Same for the other methods in this class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In cloud controller validation/check functions that raise an error end with an exclamation mark vs validation/check function that raise not an error do not end with an exclamation mark..

@kathap kathap force-pushed the azure-storage-cli-integration branch from 5c884e0 to 2596e92 Compare October 22, 2025 13:39
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