From 3dffe2aabfb0474e0800fbcce0fd96bbcc1bee11 Mon Sep 17 00:00:00 2001 From: Andrew Garner Date: Thu, 14 Nov 2024 14:02:52 -0600 Subject: [PATCH] Add skip_check_for_pending_changes option to broker This flag controls whether "update-service" fails with a broker.PendingChangesNotAppliedError or succeeds in the event that the previous manifest does not match the new manifest. This check disallows a developer from updating their service instance when an upgrade is pending - putting the control for rolling out a new version of software in the handles of the platform engineer. In many cases, a platform engineer may want to delegate the update to the developer anyway. This is a feature flag to allow the platform engineer to permit updates even if the manifest is changing and effectively an upgrade is applied. --- brokerinitiator/start.go | 1 + config/config.go | 1 + config/config_test.go | 12 ++ ...ig_with_skip_check_for_pending_changes.yml | 134 ++++++++++++++++++ task/task.go | 20 +-- task/task_test.go | 14 ++ 6 files changed, 174 insertions(+), 8 deletions(-) create mode 100644 config/test_assets/good_config_with_skip_check_for_pending_changes.yml diff --git a/brokerinitiator/start.go b/brokerinitiator/start.go index 0a9b3a6d..09e42eb5 100644 --- a/brokerinitiator/start.go +++ b/brokerinitiator/start.go @@ -73,6 +73,7 @@ func Initiate(conf config.Config, deploymentManager := task.NewDeployer(taskBoshClient, manifestGenerator, odbSecrets, boshCredhubStore, persister) deploymentManager.DisableBoshConfigs = conf.Broker.DisableBoshConfigs + deploymentManager.SkipCheckForPendingChanges = conf.Broker.SkipCheckForPendingChanges manifestSecretManager := manifestsecrets.BuildManager(conf.Broker.EnableSecureManifests, new(manifestsecrets.CredHubPathMatcher), boshCredhubStore) diff --git a/config/config.go b/config/config.go index 4092057e..a9cb6aae 100644 --- a/config/config.go +++ b/config/config.go @@ -54,6 +54,7 @@ type Broker struct { SupportBackupAgentBinding bool `yaml:"support_backup_agent_binding"` TLS TLSConfig `yaml:"tls"` EnablePersistManifest bool `yaml:"enable_persist_manifest"` + SkipCheckForPendingChanges bool `yaml:"skip_check_for_pending_changes"` } type BoshCredhub struct { diff --git a/config/config_test.go b/config/config_test.go index 814cd408..94ef2c84 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -67,6 +67,7 @@ var _ = Describe("Broker Config", func() { EnableTelemetry: true, SupportBackupAgentBinding: true, EnablePersistManifest: true, + SkipCheckForPendingChanges: false, }, Bosh: config.Bosh{ URL: "some-url", @@ -221,6 +222,17 @@ var _ = Describe("Broker Config", func() { }) }) + Context("and the config has skip_check_for_pending_changes enabled", func() { + BeforeEach(func() { + configFileName = "good_config_with_skip_check_for_pending_changes.yml" + }) + + It("returns config object", func() { + Expect(parseErr).NotTo(HaveOccurred()) + Expect(conf.Broker.SkipCheckForPendingChanges).To(BeTrue()) + }) + }) + Context("and the config has optional global resource quotas", func() { BeforeEach(func() { configFileName = "good_config_with_global_resource_quotas.yml" diff --git a/config/test_assets/good_config_with_skip_check_for_pending_changes.yml b/config/test_assets/good_config_with_skip_check_for_pending_changes.yml new file mode 100644 index 00000000..1d78f4d8 --- /dev/null +++ b/config/test_assets/good_config_with_skip_check_for_pending_changes.yml @@ -0,0 +1,134 @@ +# Copyright (C) 2016-Present Pivotal Software, Inc. All rights reserved. +# This program and the accompanying materials are made available under the terms of the under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# http://www.apache.org/licenses/LICENSE-2.0 +# Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. + +--- +broker: + port: 8080 + username: username + password: password + disable_ssl_cert_verification: true + startup_banner: false + shutdown_timeout_in_seconds: 10 + use_stdin: true + enable_telemetry: true + support_backup_agent_binding: true + enable_persist_manifest: true + skip_check_for_pending_changes: true +bosh: + url: some-url + root_ca_cert: some-cert + authentication: + basic: + username: some-username + password: some-password +cf: + url: some-cf-url + root_ca_cert: some-cf-cert + uaa: + url: a-uaa-url + client_definition: + scopes: scope1,scope2 + authorities: authority1,authority2 + authorized_grant_types: grant_type1,grant_type2 + resource_ids: resource2,resource3 + name: client_name + allowpublic: true + authentication: + user_credentials: + username: some-cf-username + password: some-cf-password +service_instances_api: + url: some-si-api-url + root_ca_cert: some-cert + authentication: + basic: + username: si-api-username + password: si-api-password +service_adapter: + path: test_assets/executable.sh +service_deployment: + releases: + - name: some-name + version: some-version + jobs: [some-job] + stemcells: + - os: ubuntu-trusty + version: 1234 +service_catalog: + id: some-id + service_name: some-marketplace-name + service_description: some-description + bindable: true + plan_updatable: true + dashboard_client: + id: "client-id-1" + secret: "secret-1" + redirect_uri: "https://dashboard.url" + metadata: + display_name: some-service-display-name + image_url: "http://test.jpg" + long_description: "Some description" + provider_display_name: "some name" + documentation_url: "some url" + support_url: "some url" + shareable: true + tags: + - some-tag + - some-other-tag + global_properties: + global_foo: global_bar + plans: + - name: some-dedicated-name + plan_id: some-dedicated-plan-id + description: I'm a dedicated plan + free: true + update: + canaries: 1 + max_in_flight: 2 + canary_watch_time: 1000-30000 + update_watch_time: 1000-30000 + serial: false + metadata: + display_name: Dedicated-Cluster + bullets: + - bullet one + - bullet two + - bullet three + costs: + - amount: + usd: 99.0 + eur: 49.0 + unit: MONTHLY + - amount: + usd: 0.99 + eur: 0.49 + unit: 1GB of messages over 20GB + quotas: + service_instance_limit: 1 + properties: + persistence: true + lifecycle_errands: + post_deploy: + - name: health-check + instances: [redis-errand/0, redis-errand/1] + pre_delete: + - name: cleanup + instances: [redis-errand/0] + instance_groups: + - name: redis-server + vm_type: some-vm + persistent_disk_type: some-disk + instances: 34 + networks: [ net1, net2 ] + - name: redis-server-2 + vm_type: some-vm-2 + instances: 3 + networks: [ net4, net5 ] + - name: redis-errand + vm_type: some-vm-3 + instances: 2 + networks: [ net5, net6 ] + lifecycle: errand diff --git a/task/task.go b/task/task.go index 4c0c6164..09d039c3 100644 --- a/task/task.go +++ b/task/task.go @@ -69,12 +69,13 @@ type ManifestPersister interface { } type Deployer struct { - boshClient BoshClient - manifestGenerator ManifestGenerator - odbSecrets ODBSecrets - bulkSetter BulkSetter - manifestPersister ManifestPersister - DisableBoshConfigs bool + boshClient BoshClient + manifestGenerator ManifestGenerator + odbSecrets ODBSecrets + bulkSetter BulkSetter + manifestPersister ManifestPersister + DisableBoshConfigs bool + SkipCheckForPendingChanges bool } func NewDeployer(boshClient BoshClient, manifestGenerator ManifestGenerator, odbSecrets ODBSecrets, bulkSetter BulkSetter, persister ManifestPersister) Deployer { @@ -194,8 +195,11 @@ func (d Deployer) Update( return 0, nil, err } } - if err := d.checkForPendingChanges(deploymentName, previousPlanID, oldManifest, oldSecretsMap, oldConfigs, logger); err != nil { - return 0, nil, err + + if !d.SkipCheckForPendingChanges { + if err := d.checkForPendingChanges(deploymentName, previousPlanID, oldManifest, oldSecretsMap, oldConfigs, logger); err != nil { + return 0, nil, err + } } generateManifestProperties := GenerateManifestProperties{ diff --git a/task/task_test.go b/task/task_test.go index 8a01d3fc..70150d90 100644 --- a/task/task_test.go +++ b/task/task_test.go @@ -852,6 +852,9 @@ var _ = Describe("Deployer", func() { Context("and there are pending changes", func() { BeforeEach(func() { manifestGenerator.GenerateManifestReturns(serviceadapter.MarshalledGenerateManifest{Manifest: "name: other-name"}, nil) + }) + + JustBeforeEach(func() { returnedTaskID, deployedManifest, deployError = deployer.Update( deploymentName, planID, @@ -893,6 +896,17 @@ var _ = Describe("Deployer", func() { Expect(deployError).To(BeAssignableToTypeOf(broker.PendingChangesNotAppliedError{})) Expect(boshClient.DeployCallCount()).To(BeZero()) }) + + When("skip_check_for_pending_changes is configured", func() { + BeforeEach(func() { + deployer.SkipCheckForPendingChanges = true + }) + + It("succeeds", func() { + Expect(deployError).ToNot(HaveOccurred()) + Expect(boshClient.DeployCallCount()).To(Equal(1)) + }) + }) }) It("fails when previous manifest contains odb managed secret syntax", func() {