Skip to content
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

[RFC0028] Enable using protected CNBs #3855

Merged
merged 4 commits into from
Jun 27, 2024

Conversation

modulo11
Copy link
Contributor

@modulo11 modulo11 commented Jun 20, 2024

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:

Building on #3778, this PR introduces the ability to use the cnb lifecycle with password protected Cloud Native Buildpacks.

  • An explanation of the use cases your change solves

  • Links to any other associated PRs

  • 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

@modulo11 modulo11 force-pushed the cnb-protected-buildpacks branch 3 times, most recently from 000999b to 035baf5 Compare June 20, 2024 11:37
pbusko and others added 3 commits June 25, 2024 08:36
Co-authored-by: Johannes Dillmann <[email protected]>
Co-authored-by: Ralf Pannemans <[email protected]>
Co-authored-by: Nicolas Bender <[email protected]>
Co-authored-by: Pavel Busko <[email protected]>
Co-authored-by: Nicolas Bender <[email protected]>
{
name: 'some-name',
relationships: { space: { data: { guid: space.guid } } },
lifecycle: { type: 'cnb', data: { buildpacks: ['http:https://buildpack.com'], credentials: { registry: { user: 'password' } } } }
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
lifecycle: { type: 'cnb', data: { buildpacks: ['http:https://buildpack.com'], credentials: { registry: { user: 'password' } } } }
lifecycle: { type: 'cnb', data: { buildpacks: ['http:https://example.com'], credentials: { registry: { user: 'password' } } } }

lifecycle_data = response_body['lifecycle']['data']

expect(response).to have_http_status :created
expect(lifecycle_data).to eq({ 'buildpacks' => ['http:https://buildpack.com'], 'stack' => 'default-stack-name', 'credentials' => '***' })
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
expect(lifecycle_data).to eq({ 'buildpacks' => ['http:https://buildpack.com'], 'stack' => 'default-stack-name', 'credentials' => '***' })
expect(lifecycle_data).to eq({ 'buildpacks' => ['http:https://example.com'], 'stack' => 'default-stack-name', 'credentials' => '***' })

@@ -1,5 +1,5 @@
require 'spec_helper'
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't that file be located at spec/unit/lib/cloud_controller/diego/buildpack/lifecycle_data_spec.rb

Copy link
Contributor

Choose a reason for hiding this comment

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

good point, the file has been moved

Copy link
Contributor

@johha johha 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 overall, just found one minor thing.

I'm a bit unsure if we should have two lifecycle_data.rb classes but also don't have a better idea at the moment

app/models/runtime/cnb_lifecycle_data_model.rb Outdated Show resolved Hide resolved
@johha johha merged commit 104568b into cloudfoundry:main Jun 27, 2024
14 checks passed
@pbusko pbusko deleted the cnb-protected-buildpacks branch June 27, 2024 20:57
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.

None yet

5 participants