Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions app/actions/buildpack_create.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
require 'repositories/buildpack_event_repository'

module VCAP::CloudController
class BuildpackCreate
class Error < ::StandardError
Expand All @@ -7,6 +9,10 @@ class Error < ::StandardError
DEFAULT_ENABLED = true
DEFAULT_LOCKED = false

def initialize(user_audit_info)
@user_audit_info = user_audit_info
end

def create(message)
Buildpack.db.transaction do
Locking[name: 'buildpacks'].lock!
Expand All @@ -22,6 +28,10 @@ def create(message)
MetadataUpdate.update(buildpack, message)

buildpack.move_to(message.position || DEFAULT_POSITION)

Repositories::BuildpackEventRepository.new.record_buildpack_create(buildpack, @user_audit_info, message.audit_hash)

buildpack
end
rescue Sequel::ValidationFailed => e
validation_error!(e, message)
Expand Down
7 changes: 7 additions & 0 deletions app/actions/buildpack_delete.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
require 'repositories/buildpack_event_repository'

module VCAP::CloudController
class BuildpackDelete
def initialize(user_audit_info)
@user_audit_info = user_audit_info
end

def delete(buildpacks)
buildpacks.each do |buildpack|
Buildpack.db.transaction do
Locking[name: 'buildpacks'].lock!
Repositories::BuildpackEventRepository.new.record_buildpack_delete(buildpack, @user_audit_info)
buildpack.destroy
end
if buildpack.key
Expand Down
8 changes: 8 additions & 0 deletions app/actions/buildpack_update.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
require 'repositories/buildpack_event_repository'

module VCAP::CloudController
class BuildpackUpdate
class Error < ::StandardError
end

def initialize(user_audit_info)
@user_audit_info = user_audit_info
end

def update(buildpack, message)
Buildpack.db.transaction do
Locking[name: 'buildpacks'].lock!
Expand All @@ -15,6 +21,8 @@ def update(buildpack, message)
buildpack.locked = message.locked if message.requested?(:locked)
buildpack.name = message.name if message.requested?(:name)
buildpack.save

Repositories::BuildpackEventRepository.new.record_buildpack_update(buildpack, @user_audit_info, message.audit_hash)
end
buildpack
rescue Sequel::ValidationFailed => e
Expand Down
6 changes: 3 additions & 3 deletions app/controllers/v3/buildpacks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def create
message = BuildpackCreateMessage.new(hashed_params[:body])
unprocessable!(message.errors.full_messages) unless message.valid?

buildpack = BuildpackCreate.new.create(message)
buildpack = BuildpackCreate.new(user_audit_info).create(message)

render status: :created, json: Presenters::V3::BuildpackPresenter.new(buildpack)
rescue BuildpackCreate::Error => e
Expand All @@ -51,7 +51,7 @@ def update
message = BuildpackUpdateMessage.new(hashed_params[:body])
unprocessable!(message.errors.full_messages) unless message.valid?

buildpack = VCAP::CloudController::BuildpackUpdate.new.update(buildpack, message)
buildpack = VCAP::CloudController::BuildpackUpdate.new(user_audit_info).update(buildpack, message)

render status: :ok, json: Presenters::V3::BuildpackPresenter.new(buildpack)
rescue BuildpackUpdate::Error => e
Expand All @@ -64,7 +64,7 @@ def destroy

unauthorized! unless permission_queryer.can_write_globally?

delete_action = BuildpackDelete.new
delete_action = BuildpackDelete.new(user_audit_info)
deletion_job = VCAP::CloudController::Jobs::DeleteActionJob.new(Buildpack, buildpack.guid, delete_action)
pollable_job = Jobs::Enqueuer.new(queue: Jobs::Queues.generic).enqueue_pollable(deletion_job)

Expand Down
62 changes: 62 additions & 0 deletions app/repositories/buildpack_event_repository.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
require 'repositories/event_types'

module VCAP::CloudController
module Repositories
class BuildpackEventRepository
def record_buildpack_create(buildpack, user_audit_info, request_attrs)
Event.create(
type: EventTypes::BUILDPACK_CREATE,
actee: buildpack.guid,
actee_type: 'buildpack',
actee_name: buildpack.name,
actor: user_audit_info.user_guid,
actor_type: 'user',
actor_name: user_audit_info.user_email,
actor_username: user_audit_info.user_name,
timestamp: Sequel::CURRENT_TIMESTAMP,
space_guid: '',
organization_guid: '',
metadata: {
request: request_attrs
}
)
end

def record_buildpack_update(buildpack, user_audit_info, request_attrs)
Event.create(
type: EventTypes::BUILDPACK_UPDATE,
actee: buildpack.guid,
actee_type: 'buildpack',
actee_name: buildpack.name,
actor: user_audit_info.user_guid,
actor_type: 'user',
actor_name: user_audit_info.user_email,
actor_username: user_audit_info.user_name,
timestamp: Sequel::CURRENT_TIMESTAMP,
space_guid: '',
organization_guid: '',
metadata: {
request: request_attrs
}
)
end

def record_buildpack_delete(buildpack, user_audit_info)
Event.create(
type: EventTypes::BUILDPACK_DELETE,
actee: buildpack.guid,
actee_type: 'buildpack',
actee_name: buildpack.name,
actor: user_audit_info.user_guid,
actor_type: 'user',
actor_name: user_audit_info.user_email,
actor_username: user_audit_info.user_name,
timestamp: Sequel::CURRENT_TIMESTAMP,
space_guid: '',
organization_guid: '',
metadata: {}
)
end
end
end
end
4 changes: 4 additions & 0 deletions app/repositories/event_types.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ class EventTypesError < StandardError
APP_SSH_AUTHORIZED = 'audit.app.ssh-authorized'.freeze,
APP_SSH_UNAUTHORIZED = 'audit.app.ssh-unauthorized'.freeze,

BUILDPACK_CREATE = 'audit.buildpack.create'.freeze,
BUILDPACK_UPDATE = 'audit.buildpack.update'.freeze,
BUILDPACK_DELETE = 'audit.buildpack.delete'.freeze,
Copy link
Member

Choose a reason for hiding this comment

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

I think BUILDPACK_UPLOAD would be a big part of the buildpack narrative we'd want to include in audit logs: https://v3-apidocs.cloudfoundry.org/version/3.209.0/index.html#upload-buildpack-bits

Copy link
Member

Choose a reason for hiding this comment

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

I'll approve this for now, but we should consider adding buildpack upload audit event in a future PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will open a new PR for it 👍


SERVICE_CREATE = 'audit.service.create'.freeze,
SERVICE_UPDATE = 'audit.service.update'.freeze,
SERVICE_DELETE = 'audit.service.delete'.freeze,
Expand Down
5 changes: 5 additions & 0 deletions docs/v3/source/includes/resources/audit_events/_header.md.erb
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ For more information, see the [Cloud Foundry docs](https://docs.cloudfoundry.org
- `audit.app.update`
- `audit.app.upload-bits`

##### Buildpack lifecycle
- `audit.buildpack.create`
- `audit.buildpack.delete`
- `audit.buildpack.update`

##### Organization lifecycle
- `audit.organization.create`
- `audit.organization.delete-request`
Expand Down
54 changes: 43 additions & 11 deletions spec/unit/actions/buildpack_create_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@
module VCAP::CloudController
RSpec.describe BuildpackCreate do
describe 'create' do
let(:user) { User.make }
let(:user_email) { '[email protected]' }
let(:user_name) { 'user-name' }
let(:user_audit_info) { UserAuditInfo.new(user_guid: user.guid, user_email: user_email, user_name: user_name) }

let!(:buildpack1) { Buildpack.create(name: 'take-up-position-1', position: 1) }
let!(:buildpack2) { Buildpack.create(name: 'take-up-position-2', position: 2) }
let!(:buildpack3) { Buildpack.create(name: 'take-up-position-3', position: 3) }
Expand All @@ -22,7 +27,7 @@ module VCAP::CloudController
locked: true,
lifecycle: Lifecycles::BUILDPACK
)
buildpack = BuildpackCreate.new.create(message)
buildpack = BuildpackCreate.new(user_audit_info).create(message)

expect(buildpack.name).to eq('the-name')
expect(buildpack.stack).to eq('the-stack')
Expand All @@ -31,6 +36,33 @@ module VCAP::CloudController
expect(buildpack.locked).to be(true)
expect(buildpack.lifecycle).to eq(Lifecycles::BUILDPACK)
end

it 'creates an audit event' do
message = BuildpackCreateMessage.new(
name: 'the-name',
stack: 'the-stack',
enabled: false,
locked: true
)
buildpack = BuildpackCreate.new(user_audit_info).create(message)

event = VCAP::CloudController::Event.last

expect(event.values).to include(
type: 'audit.buildpack.create',
actee: buildpack.guid,
actee_type: 'buildpack',
actee_name: buildpack.name,
actor: user_audit_info.user_guid,
actor_type: 'user',
actor_name: user_audit_info.user_email,
actor_username: user_audit_info.user_name,
space_guid: '',
organization_guid: ''
)
expect(event.metadata).to eq({ 'request' => message.audit_hash })
expect(event.timestamp).to be
end
end

context 'when metadata is provided' do
Expand All @@ -49,7 +81,7 @@ module VCAP::CloudController
}
}
)
buildpack = BuildpackCreate.new.create(message)
buildpack = BuildpackCreate.new(user_audit_info).create(message)

expect(buildpack.name).to eq('the-name')
expect(buildpack.stack).to eq('the-stack')
Expand All @@ -68,7 +100,7 @@ module VCAP::CloudController
name: 'the-name',
position: 2
)
buildpack = BuildpackCreate.new.create(message)
buildpack = BuildpackCreate.new(user_audit_info).create(message)

expect(buildpack.position).to eq(2)
expect(buildpack1.reload.position).to eq(1)
Expand All @@ -83,7 +115,7 @@ module VCAP::CloudController
name: 'the-name',
position: 42
)
buildpack = BuildpackCreate.new.create(message)
buildpack = BuildpackCreate.new(user_audit_info).create(message)

expect(buildpack.position).to eq(4)
end
Expand All @@ -97,7 +129,7 @@ module VCAP::CloudController
stack: 'the-stack',
locked: true
)
buildpack = BuildpackCreate.new.create(message)
buildpack = BuildpackCreate.new(user_audit_info).create(message)

expect(buildpack.enabled).to be(true)
end
Expand All @@ -110,7 +142,7 @@ module VCAP::CloudController
stack: 'the-stack',
enabled: true
)
buildpack = BuildpackCreate.new.create(message)
buildpack = BuildpackCreate.new(user_audit_info).create(message)

expect(buildpack.locked).to be(false)
end
Expand All @@ -123,7 +155,7 @@ module VCAP::CloudController
stack: 'the-stack',
lifecycle: Lifecycles::CNB
)
buildpack = BuildpackCreate.new.create(message)
buildpack = BuildpackCreate.new(user_audit_info).create(message)

expect(buildpack.lifecycle).to eq(Lifecycles::CNB)
end
Expand All @@ -138,7 +170,7 @@ module VCAP::CloudController

message = BuildpackCreateMessage.new(name: 'foobar')
expect do
BuildpackCreate.new.create(message)
BuildpackCreate.new(user_audit_info).create(message)
end.to raise_error(BuildpackCreate::Error, 'blork is busted')
end
end
Expand All @@ -148,7 +180,7 @@ module VCAP::CloudController
message = BuildpackCreateMessage.new(name: 'the-name', stack: 'does-not-exist')

expect do
BuildpackCreate.new.create(message)
BuildpackCreate.new(user_audit_info).create(message)
end.to raise_error(BuildpackCreate::Error, "Stack 'does-not-exist' does not exist")
end
end
Expand All @@ -164,7 +196,7 @@ module VCAP::CloudController
it 'raises a human-friendly error' do
message = BuildpackCreateMessage.new(name:)
expect do
BuildpackCreate.new.create(message)
BuildpackCreate.new(user_audit_info).create(message)
end.to raise_error(BuildpackCreate::Error, "Buildpack with name 'the-name' and an unassigned stack already exists")
end
end
Expand All @@ -177,7 +209,7 @@ module VCAP::CloudController
it 'raises a human-friendly error' do
message = BuildpackCreateMessage.new(name: name, stack: 'the-stack')
expect do
BuildpackCreate.new.create(message)
BuildpackCreate.new(user_audit_info).create(message)
end.to raise_error(BuildpackCreate::Error, "Buildpack with name 'the-name', stack 'the-stack' and lifecycle 'buildpack' already exists")
end
end
Expand Down
31 changes: 30 additions & 1 deletion spec/unit/actions/buildpack_delete_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@

module VCAP::CloudController
RSpec.describe BuildpackDelete do
subject(:buildpack_delete) { BuildpackDelete.new }
let(:user) { User.make }
let(:user_email) { '[email protected]' }
let(:user_name) { 'user-name' }
let(:user_audit_info) { UserAuditInfo.new(user_guid: user.guid, user_email: user_email, user_name: user_name) }

subject(:buildpack_delete) { BuildpackDelete.new(user_audit_info) }

describe '#delete' do
let!(:buildpack) { Buildpack.make }
Expand All @@ -15,6 +20,30 @@ module VCAP::CloudController
expect { buildpack.refresh }.to raise_error Sequel::Error, 'Record not found'
end

it 'creates an audit event' do
buildpack_guid = buildpack.guid
buildpack_name = buildpack.name

buildpack_delete.delete([buildpack])

event = VCAP::CloudController::Event.last

expect(event.values).to include(
type: 'audit.buildpack.delete',
actee: buildpack_guid,
actee_type: 'buildpack',
actee_name: buildpack_name,
actor: user_audit_info.user_guid,
actor_type: 'user',
actor_name: user_audit_info.user_email,
actor_username: user_audit_info.user_name,
space_guid: '',
organization_guid: ''
)
expect(event.metadata).to eq({})
expect(event.timestamp).to be
end

context 'when the buildpack has associated bits in the blobstore' do
before do
buildpack.update(key: 'the-key')
Expand Down
Loading
Loading