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

v3 should allow to update docker registry credentials #3467

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 6 additions & 1 deletion app/actions/package_update.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,14 @@ def initialize

def update(package, message)
package.db.transaction do
if package.type == PackageModel::DOCKER_TYPE && (message.username || message.password)
package.docker_username = message.username unless message.username.nil?
kathap marked this conversation as resolved.
Show resolved Hide resolved
package.docker_password = message.password unless message.password.nil?
package.save
end
MetadataUpdate.update(package, message)
end
@logger.info("Finished updating metadata on package #{package.guid}")
@logger.info("Finished updating package #{package.guid}")
package
rescue Sequel::ValidationFailed => e
raise InvalidPackage.new(e.message)
Expand Down
5 changes: 5 additions & 0 deletions app/controllers/v3/packages_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ def update

package, space = PackageFetcher.new.fetch(hashed_params[:guid])
package_not_found! unless package && permission_queryer.can_read_from_space?(space.id, space.organization_id)
unprocessable_non_docker_package_update! if package.type != PackageModel::DOCKER_TYPE && (message.username || message.password)
kathap marked this conversation as resolved.
Show resolved Hide resolved
unauthorized! unless permission_queryer.can_write_to_active_space?(space.id)
suspended! unless permission_queryer.is_space_active?(space.id)

Expand Down Expand Up @@ -212,6 +213,10 @@ def unprocessable_non_docker_package!
unprocessable!('Cannot create bits package for a Docker app.')
end

def unprocessable_non_docker_package_update!
unprocessable!('Cannot update Docker credentials for a buildpack app.')
end

def unprocessable_app!
unprocessable!('App is invalid. Ensure it exists and you have access to it.')
end
Expand Down
2 changes: 1 addition & 1 deletion app/messages/package_update_message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

module VCAP::CloudController
class PackageUpdateMessage < MetadataBaseMessage
register_allowed_keys []
register_allowed_keys %i[username password]

validates_with NoAdditionalKeysValidator
end
Expand Down
8 changes: 5 additions & 3 deletions docs/v3/source/includes/resources/packages/_update.md.erb
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,12 @@ Content-Type: application/json

#### Optional parameters

Name | Type | Description
---- | ---- | -----------
**metadata.labels** | [_label object_](#labels) | Labels applied to the package
Name | Type | Description
---- |-------------------------------------| -----------
**metadata.labels** | [_label object_](#labels) | Labels applied to the package
**metadata.annotations** | [_annotation object_](#annotations) | Annotations applied to the package
**username** | string | The username for the image’s registry. Only possible for Docker package.
**password** | string | The password for the image’s registry. Only possible for Docker package.

#### Permitted roles
|
Expand Down
58 changes: 58 additions & 0 deletions spec/unit/actions/package_update_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,63 @@ module VCAP::CloudController
expect(package).to have_annotations({ key_name: 'tokyo', value: 'grapes' })
end
end

describe 'update docker credentials' do
let(:body) do
{
username: 'Udo',
password: 'It is a secret'
}
end
let(:package) { PackageModel.make(type: 'docker', docker_image: 'image-magick.com') }
let(:message) { PackageUpdateMessage.new(body) }

it "updates the package's docker credentials" do
expect(message).to be_valid
package_update.update(package, message)

package.reload
expect(package.docker_username).to eq('Udo')
expect(package.docker_password).to eq('It is a secret')
end
end

describe 'update docker username' do
let(:body) do
{
username: 'Walz'
}
end
let(:package) { PackageModel.make(type: 'docker', docker_image: 'image-magick.com') }
let(:message) { PackageUpdateMessage.new(body) }

it "updates the package's docker username" do
expect(message).to be_valid
package_update.update(package, message)

package.reload
expect(package.docker_username).to eq('Walz')
expect(package.docker_password).to be_nil
end
end

describe 'update docker password' do
let(:body) do
{
password: 'Absolutely secret'
}
end
let(:package) { PackageModel.make(type: 'docker', docker_image: 'image-magick.com') }
let(:message) { PackageUpdateMessage.new(body) }

it "updates the package's docker password" do
expect(message).to be_valid
package_update.update(package, message)

package.reload
expect(package.docker_username).to be_nil
expect(package.docker_password).to eq('Absolutely secret')
end
end
end
end
44 changes: 44 additions & 0 deletions spec/unit/controllers/v3/packages_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,50 @@
VCAP::CloudController::AnnotationsUpdate.update(package, annotations, VCAP::CloudController::PackageAnnotationModel)
end

context 'when updating docker credentials' do
before do
set_current_user_as_admin
end

context 'but the package is bits' do
describe 'it does not update the package' do
let(:message) do
{
username: 'Walz',
password: 'Absolutely secret'
}
end

it 'fails' do
patch :update, params: { guid: package.guid }.merge(message), as: :json
expect(response).to have_http_status(:unprocessable_entity)
kathap marked this conversation as resolved.
Show resolved Hide resolved
expect(response).to have_error_message('Cannot update Docker credentials for a buildpack app.')
end
end
end

context 'and the package is docker' do
before do
package.type = 'docker'
package.save
end

describe 'it updates the package' do
let(:message) do
{
username: 'Walz',
password: 'Absolutely secret'
}
end

it 'succeeds' do
patch :update, params: { guid: package.guid }.merge(message), as: :json
expect(response).to have_http_status(:ok)
end
end
end
end

context 'when the user is an admin' do
before do
set_current_user_as_admin
Expand Down