From f8642296278a94f686f1ef07875f934c56f1fcbc Mon Sep 17 00:00:00 2001 From: Pam Artiaga <partiaga@gitlab.com> Date: Wed, 17 Apr 2024 12:53:59 +1000 Subject: [PATCH] Return or display Gitlab version if GITLAB_KAS_VERSION is a SHA Gitlab::Kas.version and Gitlab::Kas.version_info are called when the API or ~frontend needs to display the KAS semantic version. Both Gitlab::Kas.version and Gitlab::Kas.version_info bases the version on the contents of the GITLAB_KAS_VERSION file. We want to handle the possibility that the content of the GITLAB_KAS_VERSION file is a SHA. In this situation, Gitlab::Kas.version and Gitlab::Kas.version_info will be based on the Gitlab version, with the SHA as the suffix. Changelog: changed --- .../javascripts/clusters/agents/index.js | 4 +- .../clusters_list/components/agent_table.vue | 10 +-- .../clusters_list/components/agent_token.vue | 4 +- app/assets/javascripts/clusters_list/index.js | 6 +- app/helpers/clusters_helper.rb | 3 +- app/helpers/projects/cluster_agents_helper.rb | 2 +- app/views/admin/dashboard/index.html.haml | 2 +- lib/gitlab/kas.rb | 49 ++++++++++++- .../components/agent_table_spec.js | 19 +++-- .../components/agent_token_spec.js | 8 +- spec/helpers/clusters_helper_spec.rb | 5 +- .../projects/cluster_agents_helper_spec.rb | 4 +- spec/lib/gitlab/kas_spec.rb | 73 +++++++++++++++---- .../admin/dashboard/index.html.haml_spec.rb | 8 +- 14 files changed, 149 insertions(+), 48 deletions(-) diff --git a/app/assets/javascripts/clusters/agents/index.js b/app/assets/javascripts/clusters/agents/index.js index 2070a32426e4..70ad63e472e7 100644 --- a/app/assets/javascripts/clusters/agents/index.js +++ b/app/assets/javascripts/clusters/agents/index.js @@ -18,7 +18,7 @@ export default () => { emptyStateSvgPath, projectPath, kasAddress, - kasVersion, + kasInstallVersion, canAdminCluster, } = el.dataset; @@ -33,7 +33,7 @@ export default () => { emptyStateSvgPath, projectPath, kasAddress, - kasVersion, + kasInstallVersion, canAdminCluster: parseBoolean(canAdminCluster), }, render(createElement) { diff --git a/app/assets/javascripts/clusters_list/components/agent_table.vue b/app/assets/javascripts/clusters_list/components/agent_table.vue index f74555ad9e1d..ea8716c78c61 100644 --- a/app/assets/javascripts/clusters_list/components/agent_table.vue +++ b/app/assets/javascripts/clusters_list/components/agent_table.vue @@ -47,7 +47,7 @@ export default { configHelpLink: helpPagePath('user/clusters/agent/install/index', { anchor: 'create-an-agent-configuration-file', }), - inject: ['kasVersion'], + inject: ['kasCheckVersion'], props: { agents: { required: true, @@ -177,12 +177,12 @@ export default { const agentVersion = this.getAgentVersionString(agent); let allowableAgentVersion = semverInc(agentVersion, 'minor'); - const isServerPrerelease = Boolean(semverPrerelease(this.kasVersion)); + const isServerPrerelease = Boolean(semverPrerelease(this.kasCheckVersion)); if (isServerPrerelease) { allowableAgentVersion = semverInc(allowableAgentVersion, 'minor'); } - return semverLt(allowableAgentVersion, this.kasVersion); + return semverLt(allowableAgentVersion, this.kasCheckVersion); }, getVersionPopoverTitle(agent) { @@ -290,7 +290,7 @@ export default { <p class="gl-mb-0"> <gl-sprintf :message="$options.i18n.versionOutdatedText"> - <template #version>{{ kasVersion }}</template> + <template #version>{{ kasCheckVersion }}</template> </gl-sprintf> <gl-link :href="$options.versionUpdateLink" class="gl-font-sm"> {{ $options.i18n.viewDocsText }}</gl-link @@ -303,7 +303,7 @@ export default { <p v-else-if="isVersionOutdated(item)" class="gl-mb-0"> <gl-sprintf :message="$options.i18n.versionOutdatedText"> - <template #version>{{ kasVersion }}</template> + <template #version>{{ kasCheckVersion }}</template> </gl-sprintf> <gl-link :href="$options.versionUpdateLink" class="gl-font-sm"> {{ $options.i18n.viewDocsText }}</gl-link diff --git a/app/assets/javascripts/clusters_list/components/agent_token.vue b/app/assets/javascripts/clusters_list/components/agent_token.vue index 5f40815bd029..84b3f9fca2a8 100644 --- a/app/assets/javascripts/clusters_list/components/agent_token.vue +++ b/app/assets/javascripts/clusters_list/components/agent_token.vue @@ -21,7 +21,7 @@ export default { GlIcon, ModalCopyButton, }, - inject: ['kasAddress', 'kasVersion'], + inject: ['kasAddress', 'kasInstallVersion'], props: { agentName: { required: true, @@ -41,7 +41,7 @@ export default { return generateAgentRegistrationCommand({ name: this.agentName, token: this.agentToken, - version: this.kasVersion, + version: this.kasInstallVersion, address: this.kasAddress, }); }, diff --git a/app/assets/javascripts/clusters_list/index.js b/app/assets/javascripts/clusters_list/index.js index 3ceb5ac2e8c4..cee97d0562e5 100644 --- a/app/assets/javascripts/clusters_list/index.js +++ b/app/assets/javascripts/clusters_list/index.js @@ -29,7 +29,8 @@ export default () => { clustersEmptyStateImage, canAddCluster, canAdminCluster, - kasVersion, + kasInstallVersion, + kasCheckVersion, displayClusterAgents, certificateBasedClustersEnabled, } = el.dataset; @@ -47,7 +48,8 @@ export default () => { clustersEmptyStateImage, canAddCluster: parseBoolean(canAddCluster), canAdminCluster: parseBoolean(canAdminCluster), - kasVersion, + kasInstallVersion, + kasCheckVersion, displayClusterAgents: parseBoolean(displayClusterAgents), certificateBasedClustersEnabled: parseBoolean(certificateBasedClustersEnabled), }, diff --git a/app/helpers/clusters_helper.rb b/app/helpers/clusters_helper.rb index d58b8da25ea8..9371814e3701 100644 --- a/app/helpers/clusters_helper.rb +++ b/app/helpers/clusters_helper.rb @@ -26,7 +26,8 @@ def js_clusters_list_data(clusterable) default_branch_name: default_branch_name(clusterable), project_path: clusterable_project_path(clusterable), kas_address: Gitlab::Kas.external_url, - kas_version: Gitlab::Kas.version_info + kas_install_version: Gitlab::Kas.install_version_info, + kas_check_version: Gitlab::Kas.display_version_info } end diff --git a/app/helpers/projects/cluster_agents_helper.rb b/app/helpers/projects/cluster_agents_helper.rb index 7eaf3d4bbd0f..15ede867292e 100644 --- a/app/helpers/projects/cluster_agents_helper.rb +++ b/app/helpers/projects/cluster_agents_helper.rb @@ -9,7 +9,7 @@ def js_cluster_agent_details_data(agent_name, project) empty_state_svg_path: image_path('illustrations/empty-state/empty-radar-md.svg'), project_path: project.full_path, kas_address: Gitlab::Kas.external_url, - kas_version: Gitlab::Kas.version_info, + kas_install_version: Gitlab::Kas.install_version_info, can_admin_cluster: can?(current_user, :admin_cluster, project).to_s } end diff --git a/app/views/admin/dashboard/index.html.haml b/app/views/admin/dashboard/index.html.haml index 7e6dd5f62a86..54a69ec6d293 100644 --- a/app/views/admin/dashboard/index.html.haml +++ b/app/views/admin/dashboard/index.html.haml @@ -149,7 +149,7 @@ %p = _('GitLab KAS') %span.gl-float-right - = Gitlab::Kas.version + = Gitlab::Kas.display_version_info = render_if_exists 'admin/dashboard/geo' diff --git a/lib/gitlab/kas.rb b/lib/gitlab/kas.rb index de391c451215..035c0c631933 100644 --- a/lib/gitlab/kas.rb +++ b/lib/gitlab/kas.rb @@ -31,11 +31,44 @@ def ensure_secret! # # @return [String] version def version - @_version ||= Rails.root.join(VERSION_FILE).read.chomp + version_info.to_s end + # Return GitLab KAS version info + # + # @return [Gitlab::VersionInfo] version_info def version_info - Gitlab::VersionInfo.parse(version, parse_suffix: true) + return version_info_from_file if version_info_from_file.valid? + + version_info_from_gitlab_and_file_sha + end + + # Return GitLab KAS version info for display + # This is the version that is displayed on the `frontend`. This is also used to + # check if the version of an existing agent does not match the latest agent version. + # If the GITLAB_KAS_VERSION file contains a SHA, we defer instead to the Gitlab version. + # + # For further details, see: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/149794 + # + # @return [Gitlab::VersionInfo] version_info + def display_version_info + return version_info_from_file if version_info_from_file.valid? + + Gitlab.version_info + end + + # Return GitLab KAS version info for installation + # This is the version used as the image tag when generating the command to install a Gitlab agent. + # If the GITLAB_KAS_VERSION file contains a SHA, we defer instead to the Gitlab version without the patch. + # This could mean that it might point to a Gitlab agent version that is several patches behind the latest one. + # + # Further details: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/149794 + # + # @return [Gitlab::VersionInfo] version_info + def install_version_info + return version_info_from_file if version_info_from_file.valid? + + Gitlab.version_info.without_patch end # Return GitLab KAS external_url @@ -78,6 +111,18 @@ def enabled? private + def version_file_content + Rails.root.join(VERSION_FILE).read.chomp + end + + def version_info_from_file + Gitlab::VersionInfo.parse(version_file_content, parse_suffix: true) + end + + def version_info_from_gitlab_and_file_sha + Gitlab::VersionInfo.parse("#{Gitlab.version_info}+#{version_file_content}", parse_suffix: true) + end + def ssl? URI(tunnel_url).scheme === 'https' end diff --git a/spec/frontend/clusters_list/components/agent_table_spec.js b/spec/frontend/clusters_list/components/agent_table_spec.js index e3adefd86b21..d8357d1d0ef8 100644 --- a/spec/frontend/clusters_list/components/agent_table_spec.js +++ b/spec/frontend/clusters_list/components/agent_table_spec.js @@ -12,7 +12,7 @@ const defaultConfigHelpUrl = '/help/user/clusters/agent/install/index#create-an-agent-configuration-file'; const provideData = { - kasVersion: '14.8.0', + kasCheckVersion: '14.8.0', }; const defaultProps = { agents: clusterAgents, @@ -132,7 +132,7 @@ describe('AgentTable', () => { }); describe.each` - agentMockIdx | agentVersion | kasVersion | versionMismatch | versionOutdated | title + agentMockIdx | agentVersion | kasCheckVersion | versionMismatch | versionOutdated | title ${0} | ${''} | ${'14.8.0'} | ${false} | ${false} | ${''} ${1} | ${'14.8.0'} | ${'14.8.0'} | ${false} | ${false} | ${''} ${2} | ${'14.6.0'} | ${'14.8.0'} | ${false} | ${true} | ${outdatedTitle} @@ -144,8 +144,15 @@ describe('AgentTable', () => { ${8} | ${'14.8.0'} | ${'14.8.10'} | ${false} | ${false} | ${''} ${9} | ${''} | ${'14.8.0'} | ${false} | ${false} | ${''} `( - 'when agent version is "$agentVersion", KAS version is "$kasVersion" and version mismatch is "$versionMismatch"', - ({ agentMockIdx, agentVersion, kasVersion, versionMismatch, versionOutdated, title }) => { + 'when agent version is "$agentVersion", KAS version is "$kasCheckVersion" and version mismatch is "$versionMismatch"', + ({ + agentMockIdx, + agentVersion, + kasCheckVersion, + versionMismatch, + versionOutdated, + title, + }) => { const currentAgent = clusterAgents[agentMockIdx]; const findIcon = () => findVersionText(0).findComponent(GlIcon); @@ -153,12 +160,12 @@ describe('AgentTable', () => { const versionWarning = versionMismatch || versionOutdated; const outdatedText = sprintf(I18N_AGENT_TABLE.versionOutdatedText, { - version: kasVersion, + version: kasCheckVersion, }); beforeEach(() => { createWrapper({ - provide: { kasVersion }, + provide: { kasCheckVersion }, propsData: { agents: [currentAgent] }, }); }); diff --git a/spec/frontend/clusters_list/components/agent_token_spec.js b/spec/frontend/clusters_list/components/agent_token_spec.js index 9be3976fea26..8bafa11b1cb4 100644 --- a/spec/frontend/clusters_list/components/agent_token_spec.js +++ b/spec/frontend/clusters_list/components/agent_token_spec.js @@ -14,7 +14,7 @@ import CodeBlock from '~/vue_shared/components/code_block.vue'; const kasAddress = 'kas.example.com'; const agentName = 'my-agent'; const agentToken = 'agent-token'; -const kasVersion = '15.0.0'; +const kasInstallVersion = '15.0.0'; const modalId = INSTALL_AGENT_MODAL_ID; describe('InstallAgentModal', () => { @@ -30,7 +30,7 @@ describe('InstallAgentModal', () => { const createWrapper = (newAgentName = agentName) => { const provide = { kasAddress, - kasVersion, + kasInstallVersion, }; const propsData = { @@ -83,7 +83,7 @@ describe('InstallAgentModal', () => { text: generateAgentRegistrationCommand({ name: agentName, token: agentToken, - version: kasVersion, + version: kasInstallVersion, address: kasAddress, }), modalId, @@ -99,7 +99,7 @@ describe('InstallAgentModal', () => { expect(findCodeBlock().props('code')).toContain(`--namespace gitlab-agent-${agentName}`); expect(findCodeBlock().props('code')).toContain(`--set config.token=${agentToken}`); expect(findCodeBlock().props('code')).toContain(`--set config.kasAddress=${kasAddress}`); - expect(findCodeBlock().props('code')).toContain(`--set image.tag=v${kasVersion}`); + expect(findCodeBlock().props('code')).toContain(`--set image.tag=v${kasInstallVersion}`); }); it('truncates the namespace name if it exceeds the maximum length', () => { diff --git a/spec/helpers/clusters_helper_spec.rb b/spec/helpers/clusters_helper_spec.rb index 5b5b3a31fd72..8ec3535d5319 100644 --- a/spec/helpers/clusters_helper_spec.rb +++ b/spec/helpers/clusters_helper_spec.rb @@ -86,8 +86,9 @@ expect(subject[:kas_address]).to eq(Gitlab::Kas.external_url) end - it 'displays KAS version' do - expect(subject[:kas_version]).to eq(Gitlab::Kas.version_info) + it 'displays KAS versions' do + expect(subject[:kas_install_version]).to eq(Gitlab::Kas.install_version_info) + expect(subject[:kas_check_version]).to eq(Gitlab::Kas.display_version_info) end context 'user has no permissions to create a cluster' do diff --git a/spec/helpers/projects/cluster_agents_helper_spec.rb b/spec/helpers/projects/cluster_agents_helper_spec.rb index 4045a7f6bf78..d878ddf77dbe 100644 --- a/spec/helpers/projects/cluster_agents_helper_spec.rb +++ b/spec/helpers/projects/cluster_agents_helper_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::ClusterAgentsHelper do +RSpec.describe Projects::ClusterAgentsHelper, feature_category: :deployment_management do describe '#js_cluster_agent_details_data' do let_it_be(:project) { create(:project) } let_it_be(:current_user) { create(:user) } @@ -33,7 +33,7 @@ empty_state_svg_path: kind_of(String), can_admin_vulnerability: "true", kas_address: Gitlab::Kas.external_url, - kas_version: Gitlab::Kas.version_info, + kas_install_version: Gitlab::Kas.install_version_info, can_admin_cluster: "false" }) } diff --git a/spec/lib/gitlab/kas_spec.rb b/spec/lib/gitlab/kas_spec.rb index 69d9ca7d4ed3..870fde1e555e 100644 --- a/spec/lib/gitlab/kas_spec.rb +++ b/spec/lib/gitlab/kas_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::Kas do +RSpec.describe Gitlab::Kas, feature_category: :deployment_management do let(:jwt_secret) { SecureRandom.random_bytes(described_class::SECRET_LENGTH) } before do @@ -184,23 +184,70 @@ end end - describe '.version' do - it 'returns gitlab_kas version config' do - version_file = Rails.root.join(described_class::VERSION_FILE) + describe 'version information' do + it 'has valid version_infos' do + expect(described_class.version_info).to be_valid + expect(described_class.display_version_info).to be_valid + expect(described_class.install_version_info).to be_valid + end - expect(described_class.version).to eq(version_file.read.chomp) + it 'has a version based on the version_info' do + expect(described_class.version).to eq described_class.version_info.to_s end - end - describe '.version_info' do - let(:version) { '15.6.0-rc1' } + describe 'versioning according to the KAS version file content' do + before do + kas_version_file_double = instance_double(File, read: version_file_content) + allow(Rails.root).to receive(:join).with(Gitlab::Kas::VERSION_FILE).and_return(kas_version_file_double) + end - before do - allow(described_class).to receive(:version).and_return(version) - end + let(:version_file_content) { 'v16.10.1' } + + it 'has a version and version_infos based on the KAS version file' do + expected_version_string = version_file_content.sub('v', '') + + expect(described_class.version).to eq expected_version_string + expect(described_class.version_info.to_s).to eq expected_version_string + expect(described_class.display_version_info.to_s).to eq expected_version_string + expect(described_class.install_version_info.to_s).to eq expected_version_string + end + + context 'when the KAS version file content is a release candidate version' do + let(:version_file_content) { 'v16.10.1-rc42' } - it 'returns gitlab_kas version config, including suffix' do - expect(described_class.version_info.to_s).to eq(version) + it 'has a version and version_infos based on the KAS version file' do + expected_version_string = version_file_content.sub('v', '') + + expect(described_class.version).to eq expected_version_string + expect(described_class.version_info.to_s).to eq expected_version_string + expect(described_class.display_version_info.to_s).to eq expected_version_string + expect(described_class.install_version_info.to_s).to eq expected_version_string + end + end + + context 'when the KAS version file content is a SHA' do + before do + allow(Gitlab).to receive(:version_info).and_return(gitlab_version_info) + end + + let(:gitlab_version_info) { Gitlab::VersionInfo.parse('16.11.2') } + let(:version_file_content) { '5bbaac6e3d907fba9568a2e36aa1e521f589c897' } + + it 'uses the Gitlab version with the SHA as suffix' do + expected_kas_version = '16.11.2+5bbaac6e3d907fba9568a2e36aa1e521f589c897' + + expect(described_class.version_info.to_s).to eq expected_kas_version + expect(described_class.version).to eq expected_kas_version + end + + it 'uses the Gitlab version without suffix as the display_version_info' do + expect(described_class.display_version_info.to_s).to eq '16.11.2' + end + + it 'uses the Gitlab version with 0 patch version as the install_version_info' do + expect(described_class.install_version_info.to_s).to eq '16.11.0' + end + end end end diff --git a/spec/views/admin/dashboard/index.html.haml_spec.rb b/spec/views/admin/dashboard/index.html.haml_spec.rb index 9cb4d175ffc2..d98f19927458 100644 --- a/spec/views/admin/dashboard/index.html.haml_spec.rb +++ b/spec/views/admin/dashboard/index.html.haml_spec.rb @@ -72,20 +72,19 @@ end end - describe 'GitLab KAS' do + describe 'GitLab KAS', feature_category: :deployment_management do before do allow(Gitlab::Kas).to receive(:enabled?).and_return(enabled) - allow(Gitlab::Kas).to receive(:version).and_return('kas-1.2.3') end context 'KAS enabled' do let(:enabled) { true } + let(:expected_kas_version) { Gitlab::Kas.display_version_info } it 'includes KAS version' do render - expect(rendered).to have_content('GitLab KAS') - expect(rendered).to have_content('kas-1.2.3') + expect(rendered).to have_content("GitLab KAS #{expected_kas_version}") end end @@ -96,7 +95,6 @@ render expect(rendered).not_to have_content('GitLab KAS') - expect(rendered).not_to have_content('kas-1.2.3') end end end -- GitLab