Seed initial version for non-versioned terraform states

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.

This 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/-/merge_requests/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
-      "#{model.version}.tfstate"
+      if terraform_state.versioning_enabled?
+        "#{model.version}.tfstate"
+      else
+        "#{model.uuid}.tfstate"
+      end
     end
 
     def store_dir
-      Gitlab::HashedPath.new(model.uuid, root_hash: project_id)
+      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