Merge branch '235108-migrate-terraform-states-to-versioniong' into 'master'

Seed initial version for non-versioned terraform states

See merge request gitlab-org/gitlab!43665

app/models/terraform/state.rb

@@ -37,7 +37,11 @@ module Terraform
     end
 
     def latest_file
-      versioning_enabled ? latest_version&.file : file
+      if versioning_enabled?
+        latest_version&.file
+      else
+        latest_version&.file || file
+      end
     end
 
     def locked?
@@ -46,13 +50,56 @@ module Terraform
 
     def update_file!(data, version:)
       if versioning_enabled?
-        new_version = versions.build(version: version)
-        new_version.assign_attributes(created_by_user: locked_by_user, file: data)
-        new_version.save!
+        create_new_version!(data: data, version: version)
+      elsif latest_version.present?
+        migrate_legacy_version!(data: data, version: version)
       else
         self.file = data
         save!
       end
     end
+
+    private
+
+    ##
+    # If a Terraform state was created before versioning support was
+    # introduced, it will have a single version record whose file
+    # uses a legacy naming scheme in object storage. To update
+    # these states and versions to use the new behaviour, we must do
+    # the following when creating the next version:
+    #
+    #  * Read the current, non-versioned file from the old location.
+    #  * Update the :versioning_enabled flag, which determines the
+    #    naming scheme
+    #  * Resave the existing file with the updated name and location,
+    #    using a version number one prior to the new version
+    #  * Create the new version as normal
+    #
+    # This migration only needs to happen once for each state, from
+    # then on the state will behave as if it was always versioned.
+    #
+    # The code can be removed in the next major version (14.0), after
+    # which any states that haven't been migrated will need to be
+    # recreated: https://gitlab.com/gitlab-org/gitlab/-/issues/258960
+    def migrate_legacy_version!(data:, version:)
+      current_file = latest_version.file.read
+      current_version = parse_serial(current_file) || version - 1
+
+      update!(versioning_enabled: true)
+
+      reload_latest_version.update!(version: current_version, file: CarrierWaveStringFile.new(current_file))
+      create_new_version!(data: data, version: version)
+    end
+
+    def create_new_version!(data:, version:)
+      new_version = versions.build(version: version, created_by_user: locked_by_user)
+      new_version.assign_attributes(file: data)
+      new_version.save!
+    end
+
+    def parse_serial(file)
+      Gitlab::Json.parse(file)["serial"]
+    rescue JSON::ParserError
+    end
   end
 end

app/uploaders/terraform/versioned_state_uploader.rb

@@ -2,12 +2,22 @@
 
 module Terraform
   class VersionedStateUploader < StateUploader
+    delegate :terraform_state, to: :model
+
     def filename
+      if terraform_state.versioning_enabled?
         "#{model.version}.tfstate"
+      else
+        "#{model.uuid}.tfstate"
+      end
     end
 
     def store_dir
+      if terraform_state.versioning_enabled?
         Gitlab::HashedPath.new(model.uuid, root_hash: project_id)
+      else
+        project_id.to_s
+      end
     end
   end
 end

changelogs/unreleased/235108-migrate-terraform-states-to-versioniong.yml

+---
+title: Seed initial version for non-versioned terraform states
+merge_request: 43665
+author:
+type: added

db/post_migrate/20200929052138_create_initial_versions_for_pre_versioning_terraform_states.rb

+# frozen_string_literal: true
+
+class CreateInitialVersionsForPreVersioningTerraformStates < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def up
+    execute <<-SQL
+      INSERT INTO terraform_state_versions (terraform_state_id, created_at, updated_at, version, file_store, file)
+      SELECT id, NOW(), NOW(), 0, file_store, file
+      FROM terraform_states
+      WHERE versioning_enabled = FALSE
+      ON CONFLICT (terraform_state_id, version) DO NOTHING
+    SQL
+  end
+
+  def down
+  end
+end

db/schema_migrations/20200929052138

+30b84d137fcb17eaca86f1bec52d6e20c972f7083d4c983e2bb397c9126b5f0c
\ No newline at end of file

spec/migrations/20200929052138_create_initial_versions_for_pre_versioning_terraform_states_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20200929052138_create_initial_versions_for_pre_versioning_terraform_states.rb')
+
+RSpec.describe CreateInitialVersionsForPreVersioningTerraformStates do
+  let(:namespace) { table(:namespaces).create!(name: 'terraform', path: 'terraform') }
+  let(:project) { table(:projects).create!(id: 1, namespace_id: namespace.id) }
+  let(:terraform_state_versions) { table(:terraform_state_versions) }
+
+  def create_state!(project, versioning_enabled:)
+    table(:terraform_states).create!(
+      project_id: project.id,
+      uuid: 'uuid',
+      file_store: 2,
+      file: 'state.tfstate',
+      versioning_enabled: versioning_enabled
+    )
+  end
+
+  describe '#up' do
+    context 'for a state that is already versioned' do
+      let!(:terraform_state) { create_state!(project, versioning_enabled: true) }
+
+      it 'does not insert a version record' do
+        expect { migrate! }.not_to change { terraform_state_versions.count }
+      end
+    end
+
+    context 'for a state that is not yet versioned' do
+      let!(:terraform_state) { create_state!(project, versioning_enabled: false) }
+
+      it 'creates a version using the current state data' do
+        expect { migrate! }.to change { terraform_state_versions.count }.by(1)
+
+        migrated_version = terraform_state_versions.last
+        expect(migrated_version.terraform_state_id).to eq(terraform_state.id)
+        expect(migrated_version.version).to be_zero
+        expect(migrated_version.file_store).to eq(terraform_state.file_store)
+        expect(migrated_version.file).to eq(terraform_state.file)
+        expect(migrated_version.created_at).to be_present
+        expect(migrated_version.updated_at).to be_present
+      end
+    end
+  end
+end

spec/models/terraform/state_spec.rb

@@ -87,11 +87,17 @@ RSpec.describe Terraform::State do
       let(:terraform_state) { create(:terraform_state, :with_file) }
 
       it { is_expected.to eq terraform_state.file }
+
+      context 'and a version exists (migration to versioned in progress)' do
+        let!(:migrated_version) { create(:terraform_state_version, terraform_state: terraform_state) }
+
+        it { is_expected.to eq terraform_state.latest_version.file }
+      end
     end
   end
 
   describe '#update_file!' do
-    let(:version) { 2 }
+    let(:version) { 3 }
     let(:data) { Hash[terraform_version: '0.12.21'].to_json }
 
     subject { terraform_state.update_file!(CarrierWaveStringFile.new(data), version: version) }
@@ -115,6 +121,33 @@ RSpec.describe Terraform::State do
 
         expect(terraform_state.latest_file.read).to eq(data)
       end
+
+      context 'and a version exists (migration to versioned in progress)' do
+        let!(:migrated_version) { create(:terraform_state_version, terraform_state: terraform_state, version: 0) }
+
+        it 'creates a new version, corrects the migrated version number, and marks the state as versioned' do
+          expect { subject }.to change { Terraform::StateVersion.count }
+
+          expect(migrated_version.reload.version).to eq(1)
+          expect(migrated_version.file.read).to eq(terraform_state_file)
+
+          expect(terraform_state.reload.latest_version.version).to eq(version)
+          expect(terraform_state.latest_version.file.read).to eq(data)
+          expect(terraform_state).to be_versioning_enabled
+        end
+
+        context 'the current version cannot be determined' do
+          before do
+            migrated_version.update!(file: CarrierWaveStringFile.new('invalid-json'))
+          end
+
+          it 'uses version - 1 to correct the migrated version number' do
+            expect { subject }.to change { Terraform::StateVersion.count }
+
+            expect(migrated_version.reload.version).to eq(2)
+          end
+        end
+      end
     end
   end
 end

spec/uploaders/terraform/versioned_state_uploader_spec.rb

@@ -12,9 +12,18 @@ RSpec.describe Terraform::VersionedStateUploader do
   end
 
   describe '#filename' do
-    it 'contains the UUID of the terraform state record' do
+    it 'contains the version of the terraform state record' do
       expect(subject.filename).to eq("#{model.version}.tfstate")
     end
+
+    context 'legacy state with versioning disabled' do
+      let(:state) { create(:legacy_terraform_state) }
+      let(:model) { create(:terraform_state_version, terraform_state: state) }
+
+      it 'contains the UUID of the terraform state record' do
+        expect(subject.filename).to eq("#{model.uuid}.tfstate")
+      end
+    end
   end
 
   describe '#store_dir' do
@@ -25,5 +34,14 @@ RSpec.describe Terraform::VersionedStateUploader do
 
       expect(subject.store_dir).to eq(:store_dir)
     end
+
+    context 'legacy state with versioning disabled' do
+      let(:state) { create(:legacy_terraform_state) }
+      let(:model) { create(:terraform_state_version, terraform_state: state) }
+
+      it 'contains the ID of the project' do
+        expect(subject.store_dir).to include(model.project_id.to_s)
+      end
+    end
   end
 end