Merge pull request #3467 from sap-contributions/patch-v3-packages-gui…

…d-should-allow-to-update-docker-registry-credentials

v3 should allow to update docker registry credentials

app/actions/package_update.rb

@@ -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?
+          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)

app/controllers/v3/packages_controller.rb

@@ -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)
     unauthorized! unless permission_queryer.can_write_to_active_space?(space.id)
     suspended! unless permission_queryer.is_space_active?(space.id)
 
@@ -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

app/messages/package_update_message.rb

@@ -2,7 +2,7 @@
 
 module VCAP::CloudController
   class PackageUpdateMessage < MetadataBaseMessage
-    register_allowed_keys []
+    register_allowed_keys %i[username password]
 
     validates_with NoAdditionalKeysValidator
   end

docs/v3/source/includes/resources/packages/_update.md.erb

@@ -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
  |

spec/unit/actions/package_update_spec.rb

@@ -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

spec/unit/controllers/v3/packages_controller_spec.rb

@@ -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)
+            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