From e7bb7d1961955d5af4e89522e6bdaf428f3a204d Mon Sep 17 00:00:00 2001 From: Matthew John Date: Thu, 14 Mar 2024 06:19:13 +0000 Subject: [PATCH 01/20] chore: Move ModuleProviderCreate POST API endpoint parser into arg parser method and update docs Issue #513 --- docs/API.md | 12 ++++++++++++ .../server/api/terrareg_module_provider_create.py | 9 +++++++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/docs/API.md b/docs/API.md index e8de136e..d87c78ad 100644 --- a/docs/API.md +++ b/docs/API.md @@ -774,6 +774,18 @@ Provide interface to create module provider. #### POST Handle update to settings. +##### Arguments + +| Argument | Location (JSON POST body or query string argument) | Type | Required | Default | Help | +|----------|----------------------------------------------------|------|----------|---------|------| +| git_provider_id | json | str | False | `None` | ID of the git provider to associate to module provider. | +| repo_base_url_template | json | str | False | `None` | Templated base git URL. | +| repo_clone_url_template | json | str | False | `None` | Templated git clone URL. | +| repo_browse_url_template | json | str | False | `None` | Templated URL for browsing repository. | +| git_tag_format | json | str | False | `None` | Module provider git tag format. | +| git_path | json | str | False | `None` | Path within git repository that the module exists. | +| csrf_token | json | str | False | `None` | CSRF token | + ## ApiTerraregModuleProviderDelete diff --git a/terrareg/server/api/terrareg_module_provider_create.py b/terrareg/server/api/terrareg_module_provider_create.py index 565f8002..cb72911e 100644 --- a/terrareg/server/api/terrareg_module_provider_create.py +++ b/terrareg/server/api/terrareg_module_provider_create.py @@ -18,8 +18,8 @@ class ApiTerraregModuleProviderCreate(ErrorCatchingResource): terrareg.user_group_namespace_permission_type.UserGroupNamespacePermissionType.FULL, request_kwarg_map={'namespace': 'namespace'})] - def _post(self, namespace, name, provider): - """Handle update to settings.""" + def _post_arg_parser(self): + """Return arg parrser for POST request""" parser = reqparse.RequestParser() parser.add_argument( 'git_provider_id', type=str, @@ -70,6 +70,11 @@ def _post(self, namespace, name, provider): location='json', default=None ) + return parser + + def _post(self, namespace, name, provider): + """Handle update to settings.""" + parser = self._post_arg_parser() args = parser.parse_args() From 3f4c39a1a3fd98a10faa4426cc2fe6435737e0dd Mon Sep 17 00:00:00 2001 From: Matthew John Date: Thu, 14 Mar 2024 06:20:34 +0000 Subject: [PATCH 02/20] chore: Move ModuleProviderUpdate POST API endpoint parser into arg parser method and update docs Issue #513 --- docs/API.md | 16 ++++++++++++++++ .../api/terrareg_module_provider_settings.py | 8 ++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/docs/API.md b/docs/API.md index d87c78ad..b5eb0c11 100644 --- a/docs/API.md +++ b/docs/API.md @@ -810,6 +810,22 @@ Provide interface to update module provider settings. #### POST Handle update to settings. +##### Arguments + +| Argument | Location (JSON POST body or query string argument) | Type | Required | Default | Help | +|----------|----------------------------------------------------|------|----------|---------|------| +| git_provider_id | json | str | False | `None` | ID of the git provider to associate to module provider. | +| repo_base_url_template | json | str | False | `None` | Templated base git repository URL. | +| repo_clone_url_template | json | str | False | `None` | Templated git clone URL. | +| repo_browse_url_template | json | str | False | `None` | Templated URL for browsing repository. | +| git_tag_format | json | str | False | `None` | Module provider git tag format. | +| git_path | json | str | False | `None` | Path within git repository that the module exists. | +| verified | json | boolean | False | `None` | Whether module provider is marked as verified. | +| namespace | json | str | False | `None` | Name of new namespace to move module/module provider to a new namespace | +| module | json | str | False | `None` | New name of module | +| provider | json | str | False | `None` | New provider for module | +| csrf_token | json | str | False | `None` | CSRF token | + ## ApiTerraregModuleProviderIntegrations diff --git a/terrareg/server/api/terrareg_module_provider_settings.py b/terrareg/server/api/terrareg_module_provider_settings.py index 7e1860f4..1793d0aa 100644 --- a/terrareg/server/api/terrareg_module_provider_settings.py +++ b/terrareg/server/api/terrareg_module_provider_settings.py @@ -20,8 +20,8 @@ class ApiTerraregModuleProviderSettings(ErrorCatchingResource): ) ] - def _post(self, namespace, name, provider): - """Handle update to settings.""" + def _post_arg_parser(self): + """Return arg parser for POST request""" parser = reqparse.RequestParser() parser.add_argument( 'git_provider_id', type=str, @@ -100,7 +100,11 @@ def _post(self, namespace, name, provider): location='json', default=None ) + return parser + def _post(self, namespace, name, provider): + """Handle update to settings.""" + parser = self._post_arg_parser() args = parser.parse_args() terrareg.csrf.check_csrf_token(args.csrf_token) From 36a463f9369071c650ab97c8aa55d45b6133530f Mon Sep 17 00:00:00 2001 From: Matthew John Date: Thu, 14 Mar 2024 06:38:55 +0000 Subject: [PATCH 03/20] feat: Add module provider configuration to limit generated archives to only the git_path of the repository. Add column to module provider for archive_git_path. Add getter/setter methods to ModuleProvider model for the new column. Update ModuleProvider create/update APIs to set the archive_git_path attribute. Update module extractor to use the module directory (the extract directory joined with the git_path) if archive_git_path has been set for generating archives. Issue #513 --- ..._add_archive_git_path_column_to_module_.py | 28 +++++++++++++++++++ terrareg/audit_action.py | 1 + terrareg/database.py | 1 + terrareg/models.py | 18 ++++++++++++ terrareg/module_extractor.py | 21 ++++++++++++-- .../api/terrareg_module_provider_create.py | 12 +++++++- .../api/terrareg_module_provider_settings.py | 13 +++++++++ 7 files changed, 90 insertions(+), 4 deletions(-) create mode 100644 terrareg/alembic/versions/f9a80ea383cc_add_archive_git_path_column_to_module_.py diff --git a/terrareg/alembic/versions/f9a80ea383cc_add_archive_git_path_column_to_module_.py b/terrareg/alembic/versions/f9a80ea383cc_add_archive_git_path_column_to_module_.py new file mode 100644 index 00000000..770cf4a3 --- /dev/null +++ b/terrareg/alembic/versions/f9a80ea383cc_add_archive_git_path_column_to_module_.py @@ -0,0 +1,28 @@ +"""Add archive_git_path column to module provider + +Revision ID: f9a80ea383cc +Revises: 2bc9677e93d1 +Create Date: 2024-03-14 06:05:26.867657 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = 'f9a80ea383cc' +down_revision = '2bc9677e93d1' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('module_provider', sa.Column('archive_git_path', sa.Boolean(), nullable=True)) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('module_provider', 'archive_git_path') + # ### end Alembic commands ### diff --git a/terrareg/audit_action.py b/terrareg/audit_action.py index 607417c4..9b1767fd 100644 --- a/terrareg/audit_action.py +++ b/terrareg/audit_action.py @@ -16,6 +16,7 @@ class AuditAction(Enum): MODULE_PROVIDER_UPDATE_GIT_TAG_FORMAT = "module_provider_update_git_tag_format" MODULE_PROVIDER_UPDATE_GIT_PROVIDER = "module_provider_update_git_provider" MODULE_PROVIDER_UPDATE_GIT_PATH = "module_provider_update_git_path" + MODULE_PROVIDER_UPDATE_ARCHIVE_GIT_PATH = "module_provider_update_archive_git_path" MODULE_PROVIDER_UPDATE_GIT_CUSTOM_BASE_URL = "module_provider_update_git_custom_base_url" MODULE_PROVIDER_UPDATE_GIT_CUSTOM_CLONE_URL = "module_provider_update_git_custom_clone_url" MODULE_PROVIDER_UPDATE_GIT_CUSTOM_BROWSE_URL = "module_provider_update_git_custom_browse_url" diff --git a/terrareg/database.py b/terrareg/database.py index 2f96dbe1..fa184afa 100644 --- a/terrareg/database.py +++ b/terrareg/database.py @@ -453,6 +453,7 @@ def initialise(self): sqlalchemy.Column('repo_browse_url_template', sqlalchemy.String(URL_COLUMN_SIZE)), sqlalchemy.Column('git_tag_format', sqlalchemy.String(GENERAL_COLUMN_SIZE)), sqlalchemy.Column('git_path', sqlalchemy.String(URL_COLUMN_SIZE)), + sqlalchemy.Column('archive_git_path', sqlalchemy.Boolean, default=False), sqlalchemy.Column('verified', sqlalchemy.Boolean), sqlalchemy.Column( 'git_provider_id', diff --git a/terrareg/models.py b/terrareg/models.py index 0670d989..2b752cc9 100644 --- a/terrareg/models.py +++ b/terrareg/models.py @@ -2397,6 +2397,11 @@ def tag_ref_regex(self): """Return regex match for git ref to match version""" return self.get_tag_regex(self.git_ref_format) + @property + def archive_git_path(self): + """Return whether archives should only contain the contents of the git_path of the repo""" + return self._get_db_row()['archive_git_path'] + @property def git_path(self): """Return path of module within git""" @@ -2605,6 +2610,19 @@ def get_git_provider(self): return GitProvider.get(id=self._get_db_row()['git_provider_id']) return None + def update_archive_git_path(self, archive_git_path): + """Set archive_git_path value""" + original_value = self._get_db_row()['archive_git_path'] + if original_value != archive_git_path: + terrareg.audit.AuditEvent.create_audit_event( + action=terrareg.audit_action.AuditAction.MODULE_PROVIDER_UPDATE_ARCHIVE_GIT_PATH, + object_type=self.__class__.__name__, + object_id=self.id, + old_value=original_value, + new_value=archive_git_path + ) + self.update_attributes(archive_git_path=archive_git_path) + def get_git_clone_url(self): """Return URL to perform git clone""" template = None diff --git a/terrareg/module_extractor.py b/terrareg/module_extractor.py index 75ebd532..741f524c 100644 --- a/terrareg/module_extractor.py +++ b/terrareg/module_extractor.py @@ -64,6 +64,11 @@ def extract_directory(self): """Return path of extract directory.""" return self._extract_directory.name + @property + def archive_source_directory(self): + """Return directory that is used for generating the archives""" + return self.extract_directory + @property def module_directory(self): """Return path of module directory, based on configured git path.""" @@ -364,12 +369,11 @@ def tar_filter(tarinfo): return None return tarinfo - # Create tar.gz with tempfile.TemporaryDirectory(suffix='generate-archive') as temp_dir: tar_file_path = os.path.join(temp_dir, self._module_version.archive_name_tar_gz) with tarfile.open(tar_file_path, "w:gz") as tar: - tar.add(self.extract_directory, arcname='', recursive=True, filter=tar_filter) + tar.add(self.archive_source_directory, arcname='', recursive=True, filter=tar_filter) # Add tar file to file storage file_storage.upload_file(tar_file_path, self._module_version.base_directory, self._module_version.archive_name_tar_gz) @@ -384,7 +388,7 @@ def tar_filter(tarinfo): zip_file_path = os.path.join(temp_dir, self._module_version.archive_name_zip) subprocess.call( ['zip', '-r', zip_file_path, '--exclude=./.git/*', '.'], - cwd=self.extract_directory + cwd=self.archive_source_directory ) file_storage.upload_file(zip_file_path, self._module_version.base_directory, self._module_version.archive_name_zip) @@ -818,3 +822,14 @@ def process_upload(self): self._clone_repository() super(GitModuleExtractor, self).process_upload() + + @property + def archive_source_directory(self): + """Return directory that is used for generating the archives""" + # If the module provider is configured to only archive the git_path + # of the source, limit to only this path + if self._module_version.module_provider.archive_git_path: + return self.module_directory + + # Otherwise, return the root of the repository + return self.extract_directory diff --git a/terrareg/server/api/terrareg_module_provider_create.py b/terrareg/server/api/terrareg_module_provider_create.py index cb72911e..953852f6 100644 --- a/terrareg/server/api/terrareg_module_provider_create.py +++ b/terrareg/server/api/terrareg_module_provider_create.py @@ -1,5 +1,5 @@ -from flask_restful import reqparse +from flask_restful import reqparse, inputs from terrareg.server.error_catching_resource import ErrorCatchingResource import terrareg.auth_wrapper @@ -63,6 +63,14 @@ def _post_arg_parser(self): help='Path within git repository that the module exists.', location='json' ) + parser.add_argument( + 'archive_git_path', type=inputs.boolean, + required=False, + default=False, + help=('Whether to generate module archives from the git_path directory. ' + 'Otherwise, archives are generated from the root'), + location='json' + ) parser.add_argument( 'csrf_token', type=str, required=False, @@ -160,6 +168,8 @@ def _post(self, namespace, name, provider): if git_path is not None: module_provider.update_git_path(git_path=git_path) + module_provider.update_archive_git_path(archive_git_path=args.archive_git_path) + return { 'id': module_provider.id } diff --git a/terrareg/server/api/terrareg_module_provider_settings.py b/terrareg/server/api/terrareg_module_provider_settings.py index 1793d0aa..7b9ed2fd 100644 --- a/terrareg/server/api/terrareg_module_provider_settings.py +++ b/terrareg/server/api/terrareg_module_provider_settings.py @@ -65,6 +65,14 @@ def _post_arg_parser(self): help='Path within git repository that the module exists.', location='json' ) + parser.add_argument( + 'archive_git_path', type=inputs.boolean, + required=False, + default=None, + help=('Whether to generate module archives from the git_path directory. ' + 'Otherwise, archives are generated from the root'), + location='json' + ) parser.add_argument( 'verified', type=inputs.boolean, required=False, @@ -174,6 +182,11 @@ def _post(self, namespace, name, provider): if git_path is not None: module_provider.update_git_path(git_path=git_path) + # Update archive_git_path if specified + if args.archive_git_path is not None: + module_provider.update_archive_git_path(archive_git_path=args.archive_git_path) + + # Update verified if specified if args.verified is not None: module_provider.update_verified(verified=args.verified) From fb1b8175c52a7e9e28046826dc63f18f0e7dfab4 Mon Sep 17 00:00:00 2001 From: Matthew John Date: Thu, 28 Mar 2024 07:49:34 +0000 Subject: [PATCH 04/20] db: Update DB migration parent version for new archive git path after merge from main Issue #513 --- .../f9a80ea383cc_add_archive_git_path_column_to_module_.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/terrareg/alembic/versions/f9a80ea383cc_add_archive_git_path_column_to_module_.py b/terrareg/alembic/versions/f9a80ea383cc_add_archive_git_path_column_to_module_.py index 770cf4a3..430ffac8 100644 --- a/terrareg/alembic/versions/f9a80ea383cc_add_archive_git_path_column_to_module_.py +++ b/terrareg/alembic/versions/f9a80ea383cc_add_archive_git_path_column_to_module_.py @@ -1,7 +1,7 @@ """Add archive_git_path column to module provider Revision ID: f9a80ea383cc -Revises: 2bc9677e93d1 +Revises: 9dbcb4240c55 Create Date: 2024-03-14 06:05:26.867657 """ @@ -11,7 +11,7 @@ # revision identifiers, used by Alembic. revision = 'f9a80ea383cc' -down_revision = '2bc9677e93d1' +down_revision = '9dbcb4240c55' branch_labels = None depends_on = None From 4bf64b65712584fb6aa28171e0fe18e602a7b19c Mon Sep 17 00:00:00 2001 From: Matthew John Date: Sun, 16 Jun 2024 08:15:36 +0100 Subject: [PATCH 05/20] feat: Add configuration to module settings to set whether archives only contain contents of git path Issue #513 --- terrareg/models.py | 5 +++-- .../static/js/terrareg/module_provider_page.js | 14 +++++++++++++- terrareg/templates/module_provider.html | 12 ++++++++++++ 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/terrareg/models.py b/terrareg/models.py index ccbb4894..c0da6d3c 100644 --- a/terrareg/models.py +++ b/terrareg/models.py @@ -2414,9 +2414,9 @@ def tag_ref_regex(self): return self.get_tag_regex(self.git_ref_format) @property - def archive_git_path(self): + def archive_git_path(self) -> bool: """Return whether archives should only contain the contents of the git_path of the repo""" - return self._get_db_row()['archive_git_path'] + return bool(self._get_db_row()['archive_git_path']) @property def git_path(self): @@ -3066,6 +3066,7 @@ def get_terrareg_api_details(self): "git_provider_id": git_provider.pk if git_provider else None, "git_tag_format": self.git_tag_format, "git_path": self.git_path, + "archive_git_path": self.archive_git_path, "repo_base_url_template": self._get_db_row()['repo_base_url_template'], "repo_clone_url_template": self._get_db_row()['repo_clone_url_template'], "repo_browse_url_template": self._get_db_row()['repo_browse_url_template'] diff --git a/terrareg/static/js/terrareg/module_provider_page.js b/terrareg/static/js/terrareg/module_provider_page.js index b5173773..a71b5827 100644 --- a/terrareg/static/js/terrareg/module_provider_page.js +++ b/terrareg/static/js/terrareg/module_provider_page.js @@ -858,10 +858,22 @@ class SettingsTab extends ModuleDetailsTab { return; } + let config = await getConfig(); + if (this._moduleDetails.verified) { $('#settings-verified').attr('checked', true); } + // Only show "archive git path" if module hosting is enforced, + // as this will use git path for indexing and serve via modules + if (config.ALLOW_MODULE_HOSTING == "enforce") { + $('#settings-section-archive-git-path').removeClass("default-hidden"); + } + + if (this._moduleDetails.archive_git_path) { + $('#settings-archive-git-path').attr('checked', true); + } + // Check if namespace is auto-verified and, if so, show message getNamespaceDetails(this._moduleDetails.namespace).then((namespaceDetails) => { if (namespaceDetails.is_auto_verified) { @@ -870,7 +882,6 @@ class SettingsTab extends ModuleDetailsTab { }); // Setup git providers - let config = await getConfig(); let gitProviderSelect = $('#settings-git-provider'); if (config.ALLOW_CUSTOM_GIT_URL_MODULE_PROVIDER) { @@ -2479,6 +2490,7 @@ function updateModuleProviderSettings(moduleDetails) { repo_browse_url_template: gitProviderId === "" ? $('#settings-browse-url-template').val() : "", git_tag_format: $('#settings-git-tag-format').val(), git_path: $('#settings-git-path').val(), + archive_git_path: $('#settings-archive-git-path').is(':checked'), verified: $('#settings-verified').is(':checked'), csrf_token: $('#settings-csrf-token').val() }), diff --git a/terrareg/templates/module_provider.html b/terrareg/templates/module_provider.html index bbad8035..67720fd7 100644 --- a/terrareg/templates/module_provider.html +++ b/terrareg/templates/module_provider.html @@ -553,6 +553,18 @@

Module Provider

Defaults to the root of the repository.
+
+ +
+ +
+ This determines whether the generated archives only contain the contents of the Git path.
+ This is only used for providing modules from archives rather than using Git repository redirects.
+ This can be used if the source directory contains other content that you do no wish to distribute to users.
+ Ensure that there are no depdencies on Terraform outside of the "Git path", as this will not be available to users.
+ Note: This has no affect on uploaded archives. +
+
From 9cfcc0c1f628e98dd4890da9a59befc00b68c6b2 Mon Sep 17 00:00:00 2001 From: Matthew John Date: Wed, 19 Jun 2024 05:26:14 +0100 Subject: [PATCH 06/20] fix: Store git_path and archive_git_path in module version. This ensures that the configuration for a module version is retained, as it matches the indexed data. If the configuration is changed in a module, the versions will need to be re-indexed for the new path to take affect. Issue #513 --- ..._add_archive_git_path_column_to_module_.py | 27 +++++++++++++++++-- terrareg/database.py | 2 ++ terrareg/models.py | 9 +++++-- terrareg/module_extractor.py | 8 +++--- 4 files changed, 39 insertions(+), 7 deletions(-) diff --git a/terrareg/alembic/versions/f9a80ea383cc_add_archive_git_path_column_to_module_.py b/terrareg/alembic/versions/f9a80ea383cc_add_archive_git_path_column_to_module_.py index 430ffac8..b03be9dc 100644 --- a/terrareg/alembic/versions/f9a80ea383cc_add_archive_git_path_column_to_module_.py +++ b/terrareg/alembic/versions/f9a80ea383cc_add_archive_git_path_column_to_module_.py @@ -18,11 +18,34 @@ def upgrade(): # ### commands auto generated by Alembic - please adjust! ### - op.add_column('module_provider', sa.Column('archive_git_path', sa.Boolean(), nullable=True)) + with op.batch_alter_table('module_provider', schema=None) as batch_op: + batch_op.add_column(sa.Column('archive_git_path', sa.Boolean(), nullable=True)) + with op.batch_alter_table('module_version', schema=None) as batch_op: + batch_op.add_column(sa.Column('archive_git_path', sa.Boolean(), nullable=True)) + batch_op.add_column(sa.Column('git_path', sa.String(length=1024), nullable=True)) + + bind = op.get_bind() + module_provider_paths = bind.execute("""SELECT id, git_path FROM module_provider""") + for module_provider_id, git_path in module_provider_paths: + bind.execute( + sa.sql.text( + """ + UPDATE module_version + SET git_path=:git_path + WHERE module_provider_id=:module_provider_id""" + ), + git_path=git_path, + module_provider_id=module_provider_id, + ) + # ### end Alembic commands ### def downgrade(): # ### commands auto generated by Alembic - please adjust! ### - op.drop_column('module_provider', 'archive_git_path') + with op.batch_alter_table('module_provider', schema=None) as batch_op: + batch_op.drop_column('archive_git_path') + with op.batch_alter_table('module_version', schema=None) as batch_op: + batch_op.drop_column('archive_git_path') + batch_op.drop_column('git_path') # ### end Alembic commands ### diff --git a/terrareg/database.py b/terrareg/database.py index d37fd7d2..ad54226a 100644 --- a/terrareg/database.py +++ b/terrareg/database.py @@ -504,6 +504,8 @@ def initialise(self): ), sqlalchemy.Column('version', sqlalchemy.String(GENERAL_COLUMN_SIZE)), sqlalchemy.Column('git_sha', sqlalchemy.String(GENERAL_COLUMN_SIZE)), + sqlalchemy.Column('git_path', sqlalchemy.String(URL_COLUMN_SIZE)), + sqlalchemy.Column('archive_git_path', sqlalchemy.Boolean, default=False), sqlalchemy.Column( 'module_details_id', sqlalchemy.ForeignKey( diff --git a/terrareg/models.py b/terrareg/models.py index c0da6d3c..207c9503 100644 --- a/terrareg/models.py +++ b/terrareg/models.py @@ -3697,9 +3697,14 @@ def path(self): return '' @property - def git_path(self): + def git_path(self) -> Optional[str]: """Return path of module within git""" - return self._module_provider.git_path + return self._get_db_row()['git_path'] + + @property + def archive_git_path(self) -> bool: + """Return whether archives should only contain the contents of the git_path of the repo""" + return bool(self._get_db_row()['archive_git_path']) @property def id(self): diff --git a/terrareg/module_extractor.py b/terrareg/module_extractor.py index 741f524c..651a3534 100644 --- a/terrareg/module_extractor.py +++ b/terrareg/module_extractor.py @@ -72,8 +72,8 @@ def archive_source_directory(self): @property def module_directory(self): """Return path of module directory, based on configured git path.""" - if self._module_version.git_path: - return safe_join_paths(self._extract_directory.name, self._module_version.git_path) + if self._module_version.module_provider.git_path: + return safe_join_paths(self._extract_directory.name, self._module_version.module_provider.git_path) else: return self._extract_directory.name @@ -449,7 +449,9 @@ def _insert_database( published=False, git_sha=git_sha, internal=terrareg_metadata.get('internal', False), - extraction_version=EXTRACTION_VERSION + extraction_version=EXTRACTION_VERSION, + git_path=self._module_version.module_provider.git_path, + archive_git_path=self._module_version.module_provider.archive_git_path, ) def _process_submodule(self, submodule: 'terrareg.models.BaseSubmodule'): From 18f0f7f9499fd80a937328a8325aea4ed3440d3c Mon Sep 17 00:00:00 2001 From: Matthew John Date: Wed, 19 Jun 2024 05:29:02 +0100 Subject: [PATCH 07/20] fix: Ensure archive-based uploads use the git_path in the same was as git-based indexing is handled. Always show archive git path in UI, as it is used by archive upload handling and git (when module hosting is enforced) Issue #513 --- terrareg/module_extractor.py | 17 ++++++----------- .../static/js/terrareg/module_provider_page.js | 6 ------ terrareg/templates/module_provider.html | 4 ++-- 3 files changed, 8 insertions(+), 19 deletions(-) diff --git a/terrareg/module_extractor.py b/terrareg/module_extractor.py index 651a3534..a22d56bf 100644 --- a/terrareg/module_extractor.py +++ b/terrareg/module_extractor.py @@ -67,6 +67,12 @@ def extract_directory(self): @property def archive_source_directory(self): """Return directory that is used for generating the archives""" + # If the module provider is configured to only archive the git_path + # of the source, limit to only this path + if self._module_version.module_provider.archive_git_path: + return self.module_directory + + # Otherwise, return the root of the repository return self.extract_directory @property @@ -824,14 +830,3 @@ def process_upload(self): self._clone_repository() super(GitModuleExtractor, self).process_upload() - - @property - def archive_source_directory(self): - """Return directory that is used for generating the archives""" - # If the module provider is configured to only archive the git_path - # of the source, limit to only this path - if self._module_version.module_provider.archive_git_path: - return self.module_directory - - # Otherwise, return the root of the repository - return self.extract_directory diff --git a/terrareg/static/js/terrareg/module_provider_page.js b/terrareg/static/js/terrareg/module_provider_page.js index a71b5827..02e2ce92 100644 --- a/terrareg/static/js/terrareg/module_provider_page.js +++ b/terrareg/static/js/terrareg/module_provider_page.js @@ -864,12 +864,6 @@ class SettingsTab extends ModuleDetailsTab { $('#settings-verified').attr('checked', true); } - // Only show "archive git path" if module hosting is enforced, - // as this will use git path for indexing and serve via modules - if (config.ALLOW_MODULE_HOSTING == "enforce") { - $('#settings-section-archive-git-path').removeClass("default-hidden"); - } - if (this._moduleDetails.archive_git_path) { $('#settings-archive-git-path').attr('checked', true); } diff --git a/terrareg/templates/module_provider.html b/terrareg/templates/module_provider.html index 67720fd7..ec5bb225 100644 --- a/terrareg/templates/module_provider.html +++ b/terrareg/templates/module_provider.html @@ -553,8 +553,8 @@

Module Provider

Defaults to the root of the repository.
-
- +
+
From 9fc63813cdcd7e52e87f203f859b997c4dd673ad Mon Sep 17 00:00:00 2001 From: Matthew John Date: Wed, 19 Jun 2024 05:29:45 +0100 Subject: [PATCH 08/20] fix: Fix use of git_path for archive-based responses and git-based responses when downloading a module version. The git path is always included, unless an archive with archive_git_path is provided Issue #513 --- terrareg/models.py | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/terrareg/models.py b/terrareg/models.py index 207c9503..37fcfce8 100644 --- a/terrareg/models.py +++ b/terrareg/models.py @@ -3916,7 +3916,7 @@ def get_git_clone_url(self): return None - def get_source_download_url(self, request_domain: str, direct_http_request: bool, path: Optional[bool]=None): + def get_source_download_url(self, request_domain: str, direct_http_request: bool, path: Optional[str]=None): """Return URL to download source file.""" rendered_url = None @@ -3938,12 +3938,12 @@ def get_source_download_url(self, request_domain: str, direct_http_request: bool # Check if git_path has been set and prepend to path, if set. path = os.path.join(self.git_path or '', path or '') - # Remove any trailing slashes from path - if path and path.endswith('/'): - path = path[:-1] - - # Check if path is present for module (only used for submodules) + # Check if path is present for module if path: + + # Remove any trailing slashes from path + path = path.lstrip('/').rstrip('/') + rendered_url = '{rendered_url}//{path}'.format( rendered_url=rendered_url, path=path) @@ -3965,6 +3965,25 @@ def get_source_download_url(self, request_domain: str, direct_http_request: bool if config.ALLOW_MODULE_HOSTING is not terrareg.config.ModuleHostingMode.DISALLOW: url = '/v1/terrareg/modules/{0}/{1}'.format(self.id, self.archive_name_zip) + # If archive does not contain just the git_path, + # check if git_path has been set and prepend to path, if set. + if not self.archive_git_path: + path = os.path.join(self.git_path or '', path or '') + + # Check if path is present for module + if path: + # Remove any trailing slashes from path + path = path.lstrip('/').rstrip('/') + + rendered_url = '{rendered_url}//{path}'.format( + rendered_url=rendered_url, + path=path) + if path: + url = '{url}//{path}'.format( + url=url, + path=path + ) + # If authentication is required, generate pre-signed URL if not config.ALLOW_UNAUTHENTICATED_ACCESS: presign_key = TerraformSourcePresignedUrl.generate_presigned_key(url=url) From 063a0f002eaa7abaadc4d5a82762e9bc5b3a90e7 Mon Sep 17 00:00:00 2001 From: Matthew John Date: Wed, 19 Jun 2024 05:30:38 +0100 Subject: [PATCH 09/20] fix: Throw error when git_path cannot be found during extraction Issue #513 --- terrareg/module_extractor.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/terrareg/module_extractor.py b/terrareg/module_extractor.py index a22d56bf..d26ca21d 100644 --- a/terrareg/module_extractor.py +++ b/terrareg/module_extractor.py @@ -672,6 +672,11 @@ def _extract_description(self, readme_content): def process_upload(self): """Handle data extraction from module source.""" + + # Ensure base directory exists + if not os.path.isdir(self.module_directory): + raise PathDoesNotExistError(f"Base module could not be found (git path: {self._module_version.git_path})") + # Generate the archive, unless the module has a git clone URL and # the config for deleting externally hosted artifacts is enabled. # Always perform this first before making any modifications to the repo From 88d027c886f50db3b67cd67dbf24125e0be86b3a Mon Sep 17 00:00:00 2001 From: Matthew John Date: Wed, 19 Jun 2024 06:10:36 +0100 Subject: [PATCH 10/20] test: Update tests with new archive_git_path parameter and add tests for ensuring creation of archive files respects archive_git_path Issue #513 --- .../module_extractor/test_process_upload.py | 55 +++++++++++++++++-- test/unit/terrareg/__init__.py | 7 ++- ...st_api_terrareg_module_provider_details.py | 11 +++- ...est_api_terrareg_module_version_details.py | 6 ++ 4 files changed, 70 insertions(+), 9 deletions(-) diff --git a/test/integration/terrareg/module_extractor/test_process_upload.py b/test/integration/terrareg/module_extractor/test_process_upload.py index 1c21eed0..d15a236d 100644 --- a/test/integration/terrareg/module_extractor/test_process_upload.py +++ b/test/integration/terrareg/module_extractor/test_process_upload.py @@ -610,7 +610,11 @@ def test_all_features(self): }, ] - def test_non_root_repo_directory(self): + @pytest.mark.parametrize('archive_git_path', [ + True, + False + ]) + def test_non_root_repo_directory(self, archive_git_path): """Test uploading a module within a sub-directory of a module.""" test_upload = UploadTestModule() @@ -619,15 +623,15 @@ def test_non_root_repo_directory(self): module_provider = ModuleProvider.get(module=module, name='test', create=True) module_provider.update_git_provider(GitProvider(2)) - module_provider.update_git_path('subdirectory/in/repo') + module_provider.update_git_path('subdirectory/in/{namespace}-{module}-{provider}') + module_provider.update_archive_git_path(archive_git_path) module_version = ModuleVersion(module_provider=module_provider, version='1.1.0') module_version.prepare_module() with test_upload as zip_file: with test_upload as upload_directory: - - module_dir = os.path.join(upload_directory, 'subdirectory/in/repo') + module_dir = os.path.join(upload_directory, 'subdirectory/in/testprocessupload-git-path-test') os.makedirs(module_dir) # Create main.tf @@ -664,6 +668,37 @@ def test_non_root_repo_directory(self): UploadTestModule.upload_module_version(module_version=module_version, zip_file=zip_file) + # Check contents of archive + file_storage = terrareg.file_storage.FileStorageFactory().get_file_storage() + zip_file = file_storage.read_file(module_version.archive_path_zip, bytes_mode=True) + with zipfile.ZipFile(zip_file) as z: + zip_contents = [ + fileobj.filename + for fileobj in z.infolist() + if not fileobj.is_dir() + ] + + if archive_git_path: + assert zip_contents == [ + 'README.md', + 'modules/testmodule2/main.tf', + 'modules/testmodule1/main.tf', + 'terrareg.json', + 'main.tf', + 'examples/testexample2/main.tf', + 'examples/testexample1/main.tf', + ] + else: + assert zip_contents == [ + 'subdirectory/in/testprocessupload-git-path-test/README.md', + 'subdirectory/in/testprocessupload-git-path-test/modules/testmodule2/main.tf', + 'subdirectory/in/testprocessupload-git-path-test/modules/testmodule1/main.tf', + 'subdirectory/in/testprocessupload-git-path-test/terrareg.json', + 'subdirectory/in/testprocessupload-git-path-test/main.tf', + 'subdirectory/in/testprocessupload-git-path-test/examples/testexample2/main.tf', + 'subdirectory/in/testprocessupload-git-path-test/examples/testexample1/main.tf', + ] + # Ensure README is present in module version assert module_version.get_readme_content() == UploadTestModule.TEST_README_CONTENT @@ -700,6 +735,18 @@ def test_non_root_repo_directory(self): assert module_version.description == 'Test unittest description' assert module_version.owner == 'Test unittest owner' + with mock.patch('terrareg.config.Config.ALLOW_MODULE_HOSTING', terrareg.config.ModuleHostingMode.ENFORCE): + if archive_git_path: + assert module_version.get_source_download_url(request_domain='localhost', direct_http_request=True) == 'https://localhost:443/v1/terrareg/modules/testprocessupload/git-path/test/1.1.0/source.zip' + else: + assert module_version.get_source_download_url(request_domain='localhost', direct_http_request=True) == 'https://localhost:443/v1/terrareg/modules/testprocessupload/git-path/test/1.1.0/source.zip//subdirectory/in/testprocessupload-git-path-test' + + # Using git path, URL should always contain the sub-path + if archive_git_path: + assert module_version.get_source_download_url(request_domain='localhost', direct_http_request=True) == 'git::ssh://clone-url.com/testprocessupload/git-path-test//subdirectory/in/testprocessupload-git-path-test?ref=1.1.0' + else: + assert module_version.get_source_download_url(request_domain='localhost', direct_http_request=True) == 'git::ssh://clone-url.com/testprocessupload/git-path-test//subdirectory/in/testprocessupload-git-path-test?ref=1.1.0' + def test_uploading_module_with_invalid_terraform(self): """Test uploading a module with invalid terraform.""" test_upload = UploadTestModule() diff --git a/test/unit/terrareg/__init__.py b/test/unit/terrareg/__init__.py index 85fd2e65..cba3fe10 100644 --- a/test/unit/terrareg/__init__.py +++ b/test/unit/terrareg/__init__.py @@ -271,7 +271,9 @@ def _get_db_row(self): 'published': unittest_data.get('published', False), 'beta': unittest_data.get('beta', False), 'module_details_id': unittest_data.get('module_details_id', None), - 'extraction_version': unittest_data.get('extraction_version', EXTRACTION_VERSION) + 'extraction_version': unittest_data.get('extraction_version', EXTRACTION_VERSION), + 'git_path': unittest_data.get('git_path', None), + 'archive_git_path': unittest_data.get('archive_git_path', False), } mock_method(request, 'terrareg.models.ModuleVersion._get_db_row', _get_db_row) @@ -385,7 +387,8 @@ def _get_db_row(self): 'repo_browse_url_template': data.get('repo_browse_url_template', None), 'git_provider_id': data.get('git_provider_id', None), 'git_tag_format': data.get('git_tag_format', None), - 'git_path': data.get('git_path', None) + 'git_path': data.get('git_path', None), + 'archive_git_path': data.get('archive_git_path', False), } mock_method(request, 'terrareg.models.ModuleProvider._get_db_row', _get_db_row) diff --git a/test/unit/terrareg/server/test_api_terrareg_module_provider_details.py b/test/unit/terrareg/server/test_api_terrareg_module_provider_details.py index 9b0215d5..e63a8d74 100644 --- a/test/unit/terrareg/server/test_api_terrareg_module_provider_details.py +++ b/test/unit/terrareg/server/test_api_terrareg_module_provider_details.py @@ -50,6 +50,7 @@ def test_existing_module_provider_no_custom_urls(self, client, mock_models): 'security_failures': 0, 'security_results': None, 'git_path': None, + 'archive_git_path': False, 'additional_tab_files': {}, 'graph_url': '/modules/testnamespace/lonelymodule/testprovider/1.0.0/graph', 'module_extraction_up_to_date': True, @@ -175,6 +176,7 @@ def test_existing_module_provider_with_security_issues( 'security_failures': expected_security_issues, 'security_results': expected_security_results, 'git_path': None, + 'archive_git_path': False, 'additional_tab_files': {}, 'graph_url': '/modules/testnamespace/withsecurityissues/testprovider/1.0.0/graph', 'module_extraction_up_to_date': True, @@ -223,7 +225,8 @@ def test_existing_module_provider_with_git_provider_and_no_versions(self, client 'repo_browse_url_template': None, 'repo_clone_url_template': None, 'versions': [], - 'git_path': None + 'git_path': None, + 'archive_git_path': False, } assert res.status_code == 200 @@ -250,7 +253,8 @@ def test_existing_module_provider_with_custom_repo_urls_and_unpublished_version( 'repo_browse_url_template': 'https://custom-localhost.com/{namespace}/{module}-{provider}/browse/{tag}/{path}', 'repo_clone_url_template': 'ssh://custom-localhost.com/{namespace}/{module}-{provider}', 'versions': [], - 'git_path': None + 'git_path': None, + 'archive_git_path': False, } assert res.status_code == 200 @@ -278,7 +282,8 @@ def test_existing_module_provider_with_no_git_provider_or_custom_urls_and_only_b 'repo_browse_url_template': None, 'repo_clone_url_template': None, 'versions': [], - 'git_path': None + 'git_path': None, + 'archive_git_path': False, } assert res.status_code == 200 diff --git a/test/unit/terrareg/server/test_api_terrareg_module_version_details.py b/test/unit/terrareg/server/test_api_terrareg_module_version_details.py index 0b4641a7..0f4e4f9a 100644 --- a/test/unit/terrareg/server/test_api_terrareg_module_version_details.py +++ b/test/unit/terrareg/server/test_api_terrareg_module_version_details.py @@ -52,6 +52,7 @@ def test_existing_module_version_no_custom_urls(self, client, mock_models): 'security_failures': 0, 'security_results': None, 'git_path': None, + 'archive_git_path': False, 'additional_tab_files': {}, 'graph_url': '/modules/testnamespace/lonelymodule/testprovider/1.0.0/graph', 'module_extraction_up_to_date': True, @@ -84,6 +85,7 @@ def test_existing_fully_populated_module(self, client, mock_models): "git_provider_id": None, "git_tag_format": "{version}", "git_path": None, + 'archive_git_path': False, "repo_base_url_template": "https://mp-base-url.com/{namespace}/{module}-{provider}", "repo_clone_url_template": "ssh://mp-clone-url.com/{namespace}/{module}-{provider}", "repo_browse_url_template": "https://mp-browse-url.com/{namespace}/{module}-{provider}/browse/{tag}/{path}suffix", @@ -245,6 +247,7 @@ def test_html_output(self, client, mock_models): "git_provider_id": None, "git_tag_format": "{version}", "git_path": None, + 'archive_git_path': False, "repo_base_url_template": "https://mp-base-url.com/{namespace}/{module}-{provider}", "repo_clone_url_template": "ssh://mp-clone-url.com/{namespace}/{module}-{provider}", "repo_browse_url_template": "https://mp-browse-url.com/{namespace}/{module}-{provider}/browse/{tag}/{path}suffix", @@ -460,6 +463,7 @@ def test_existing_module_version_with_git_provider(self, client, mock_models): 'security_failures': 0, 'security_results': None, 'git_path': None, + 'archive_git_path': False, 'additional_tab_files': {}, 'graph_url': '/modules/moduleextraction/gitextraction/usesgitproviderwithversions/2.2.2/graph', 'module_extraction_up_to_date': True, @@ -532,6 +536,7 @@ def test_existing_module_version_with_custom_repo_urls_and_unpublished_version(s 'security_failures': 0, 'security_results': None, 'git_path': None, + 'archive_git_path': False, 'additional_tab_files': {}, 'graph_url': '/modules/testnamespace/modulenotpublished/testprovider/10.2.1/graph', 'module_extraction_up_to_date': True, @@ -608,6 +613,7 @@ def test_existing_module_version_with_no_git_provider_or_custom_urls_and_only_be 'security_failures': 0, 'security_results': None, 'git_path': None, + 'archive_git_path': False, 'additional_tab_files': {}, 'graph_url': '/modules/testnamespace/onlybeta/testprovider/2.2.4-beta/graph', 'module_extraction_up_to_date': True, From 2104038e85c224b9e596d9ec038a2c8b27d32a81 Mon Sep 17 00:00:00 2001 From: Matthew John Date: Wed, 19 Jun 2024 06:17:53 +0100 Subject: [PATCH 11/20] fix: Update description of git_path and archive_git_path to ensure it makes sense with new behavior Issue #513 --- terrareg/templates/module_provider.html | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/terrareg/templates/module_provider.html b/terrareg/templates/module_provider.html index ec5bb225..8c536144 100644 --- a/terrareg/templates/module_provider.html +++ b/terrareg/templates/module_provider.html @@ -545,12 +545,12 @@

Module Provider

- +
- Set the path within the repository that the module exists.
- Defaults to the root of the repository.
+ Set the path within the repository/archive that the module exists.
+ Defaults to the root of the repository/archive.
@@ -558,11 +558,10 @@

Module Provider

- This determines whether the generated archives only contain the contents of the Git path.
+ This determines whether the generated archives only contain the contents of the Module path.
This is only used for providing modules from archives rather than using Git repository redirects.
This can be used if the source directory contains other content that you do no wish to distribute to users.
- Ensure that there are no depdencies on Terraform outside of the "Git path", as this will not be available to users.
- Note: This has no affect on uploaded archives. + Ensure that there are no depdencies on Terraform outside of the "Module path", as this will not be available to users.
From eee038656293c9c388b00302dc71888ba4ccc29f Mon Sep 17 00:00:00 2001 From: Matthew John Date: Wed, 19 Jun 2024 06:32:38 +0100 Subject: [PATCH 12/20] test: Add tests to ensure archive git path can be set in UI Issue #513 --- test/selenium/test_module_provider.py | 36 ++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/test/selenium/test_module_provider.py b/test/selenium/test_module_provider.py index 0e57f97f..5bcd2153 100644 --- a/test/selenium/test_module_provider.py +++ b/test/selenium/test_module_provider.py @@ -1798,13 +1798,14 @@ def test_delete_module_version(self, mock_create_audit_event): assert ModuleVersion.get(module_provider=module_provider, version='2.5.5') is None def test_git_path_setting(self): - """Test setting git tag in module provider settings.""" + """Test setting git path in module provider settings.""" self.perform_admin_authentication(password='unittest-password') # Ensure user is redirected to module page self.selenium_instance.get(self.get_url('/modules/moduledetails/fullypopulated/testprovider#settings')) + self.wait_for_element(By.ID, 'module-tab-link-settings') - settings_input = self._get_settings_field_by_label('Git path') + settings_input = self._get_settings_field_by_label('Module path') assert settings_input.get_attribute('value') == '' # Enter git path @@ -1815,7 +1816,8 @@ def test_git_path_setting(self): assert module_provider.git_path == 'test/sub/directory' self.selenium_instance.refresh() - settings_input = self._get_settings_field_by_label('Git path') + self.wait_for_element(By.ID, 'module-tab-link-settings') + settings_input = self._get_settings_field_by_label('Module path') assert settings_input.get_attribute('value') == 'test/sub/directory' settings_input.clear() @@ -1823,6 +1825,34 @@ def test_git_path_setting(self): module_provider._cache_db_row = None assert module_provider.git_path == None + def test_archive_git_path_setting(self): + """Test setting archive git path in module provider settings.""" + self.perform_admin_authentication(password='unittest-password') + + # Ensure user is redirected to module page + self.selenium_instance.get(self.get_url('/modules/moduledetails/fullypopulated/testprovider#settings')) + self.wait_for_element(By.ID, 'module-tab-link-settings') + + settings_input = self._get_settings_field_by_label('Only include module path in archive') + assert settings_input.get_attribute('checked') == None + + # Enter git path + settings_input.click() + self._click_save_settings() + + module_provider = ModuleProvider(Module(Namespace('moduledetails'), 'fullypopulated'), 'testprovider') + assert module_provider.archive_git_path is True + + self.selenium_instance.refresh() + self.wait_for_element(By.ID, 'module-tab-link-settings') + settings_input = self._get_settings_field_by_label('Only include module path in archive') + assert settings_input.get_attribute('checked') == 'true' + settings_input.click() + + self._click_save_settings() + module_provider._cache_db_row = None + assert module_provider.archive_git_path is False + def test_updating_module_name(self): """Test changing module name in module provider settings""" self.perform_admin_authentication(password="unittest-password") From 2fac34890e9e0e2debd4c8af4f19108432fc40d2 Mon Sep 17 00:00:00 2001 From: Matthew John Date: Fri, 21 Jun 2024 07:38:52 +0100 Subject: [PATCH 13/20] test: Revert changes to module config in test_non_root_repo_directory to avoid affecting other tests and sort list of expected files in archive Issue #513 --- .../module_extractor/test_process_upload.py | 208 +++++++++--------- 1 file changed, 108 insertions(+), 100 deletions(-) diff --git a/test/integration/terrareg/module_extractor/test_process_upload.py b/test/integration/terrareg/module_extractor/test_process_upload.py index d15a236d..2641ff47 100644 --- a/test/integration/terrareg/module_extractor/test_process_upload.py +++ b/test/integration/terrareg/module_extractor/test_process_upload.py @@ -626,126 +626,134 @@ def test_non_root_repo_directory(self, archive_git_path): module_provider.update_git_path('subdirectory/in/{namespace}-{module}-{provider}') module_provider.update_archive_git_path(archive_git_path) - module_version = ModuleVersion(module_provider=module_provider, version='1.1.0') - module_version.prepare_module() + try: - with test_upload as zip_file: - with test_upload as upload_directory: - module_dir = os.path.join(upload_directory, 'subdirectory/in/testprocessupload-git-path-test') - os.makedirs(module_dir) + module_version = ModuleVersion(module_provider=module_provider, version='1.1.0') + module_version.prepare_module() - # Create main.tf - with open(os.path.join(module_dir, 'main.tf'), 'w') as main_tf_fh: - main_tf_fh.writelines(UploadTestModule.VALID_MAIN_TF_FILE) + with test_upload as zip_file: + with test_upload as upload_directory: + module_dir = os.path.join(upload_directory, 'subdirectory/in/testprocessupload-git-path-test') + os.makedirs(module_dir) - with open(os.path.join(module_dir, 'terrareg.json'), 'w') as metadata_fh: - metadata_fh.writelines(json.dumps({ - 'description': 'Test unittest description', - 'owner': 'Test unittest owner' - })) + # Create main.tf + with open(os.path.join(module_dir, 'main.tf'), 'w') as main_tf_fh: + main_tf_fh.writelines(UploadTestModule.VALID_MAIN_TF_FILE) - # Create README - with open(os.path.join(module_dir, 'README.md'), 'w') as main_tf_fh: - main_tf_fh.writelines(UploadTestModule.TEST_README_CONTENT) + with open(os.path.join(module_dir, 'terrareg.json'), 'w') as metadata_fh: + metadata_fh.writelines(json.dumps({ + 'description': 'Test unittest description', + 'owner': 'Test unittest owner' + })) - os.mkdir(os.path.join(module_dir, 'modules')) + # Create README + with open(os.path.join(module_dir, 'README.md'), 'w') as main_tf_fh: + main_tf_fh.writelines(UploadTestModule.TEST_README_CONTENT) - # Create main.tf in each of the submodules - for itx in [1, 2]: - root_dir = os.path.join(module_dir, 'modules', 'testmodule{itx}'.format(itx=itx)) - os.mkdir(root_dir) - with open(os.path.join(root_dir, 'main.tf'), 'w') as main_tf_fh: - main_tf_fh.writelines(UploadTestModule.SUB_MODULE_MAIN_TF.format(itx=itx)) + os.mkdir(os.path.join(module_dir, 'modules')) - os.mkdir(os.path.join(module_dir, 'examples')) + # Create main.tf in each of the submodules + for itx in [1, 2]: + root_dir = os.path.join(module_dir, 'modules', 'testmodule{itx}'.format(itx=itx)) + os.mkdir(root_dir) + with open(os.path.join(root_dir, 'main.tf'), 'w') as main_tf_fh: + main_tf_fh.writelines(UploadTestModule.SUB_MODULE_MAIN_TF.format(itx=itx)) - # Create main.tf in each of the examples - for itx in [1, 2]: - root_dir = os.path.join(module_dir, 'examples', 'testexample{itx}'.format(itx=itx)) - os.mkdir(root_dir) - with open(os.path.join(root_dir, 'main.tf'), 'w') as main_tf_fh: - main_tf_fh.writelines(UploadTestModule.SUB_MODULE_MAIN_TF.format(itx=itx)) + os.mkdir(os.path.join(module_dir, 'examples')) - UploadTestModule.upload_module_version(module_version=module_version, zip_file=zip_file) + # Create main.tf in each of the examples + for itx in [1, 2]: + root_dir = os.path.join(module_dir, 'examples', 'testexample{itx}'.format(itx=itx)) + os.mkdir(root_dir) + with open(os.path.join(root_dir, 'main.tf'), 'w') as main_tf_fh: + main_tf_fh.writelines(UploadTestModule.SUB_MODULE_MAIN_TF.format(itx=itx)) - # Check contents of archive - file_storage = terrareg.file_storage.FileStorageFactory().get_file_storage() - zip_file = file_storage.read_file(module_version.archive_path_zip, bytes_mode=True) - with zipfile.ZipFile(zip_file) as z: - zip_contents = [ - fileobj.filename - for fileobj in z.infolist() - if not fileobj.is_dir() - ] + UploadTestModule.upload_module_version(module_version=module_version, zip_file=zip_file) - if archive_git_path: - assert zip_contents == [ - 'README.md', - 'modules/testmodule2/main.tf', - 'modules/testmodule1/main.tf', - 'terrareg.json', - 'main.tf', - 'examples/testexample2/main.tf', - 'examples/testexample1/main.tf', - ] - else: - assert zip_contents == [ - 'subdirectory/in/testprocessupload-git-path-test/README.md', - 'subdirectory/in/testprocessupload-git-path-test/modules/testmodule2/main.tf', - 'subdirectory/in/testprocessupload-git-path-test/modules/testmodule1/main.tf', - 'subdirectory/in/testprocessupload-git-path-test/terrareg.json', - 'subdirectory/in/testprocessupload-git-path-test/main.tf', - 'subdirectory/in/testprocessupload-git-path-test/examples/testexample2/main.tf', - 'subdirectory/in/testprocessupload-git-path-test/examples/testexample1/main.tf', + # Check contents of archive + file_storage = terrareg.file_storage.FileStorageFactory().get_file_storage() + zip_file = file_storage.read_file(module_version.archive_path_zip, bytes_mode=True) + with zipfile.ZipFile(zip_file) as z: + zip_contents = [ + fileobj.filename + for fileobj in z.infolist() + if not fileobj.is_dir() ] - # Ensure README is present in module version - assert module_version.get_readme_content() == UploadTestModule.TEST_README_CONTENT + if archive_git_path: + assert sorted(zip_contents) == [ + 'README.md', + 'examples/testexample1/main.tf', + 'examples/testexample2/main.tf', + 'main.tf', + 'modules/testmodule1/main.tf', + 'modules/testmodule2/main.tf', + 'terrareg.json' + ] - # Ensure terraform docs output contains variable and output - assert module_version.get_terraform_inputs() == [ - { - 'default': 'test_default_val', - 'description': 'This is a test input', - 'name': 'test_input', - 'required': False, - 'type': 'string' - } - ] - assert module_version.get_terraform_outputs() == [ - { - 'description': 'test output', - 'name': 'test_output' - } - ] + else: + assert sorted(zip_contents) == [ + 'subdirectory/in/testprocessupload-git-path-test/README.md', + 'subdirectory/in/testprocessupload-git-path-test/examples/testexample1/main.tf', + 'subdirectory/in/testprocessupload-git-path-test/examples/testexample2/main.tf', + 'subdirectory/in/testprocessupload-git-path-test/main.tf', + 'subdirectory/in/testprocessupload-git-path-test/modules/testmodule1/main.tf', + 'subdirectory/in/testprocessupload-git-path-test/modules/testmodule2/main.tf', + 'subdirectory/in/testprocessupload-git-path-test/terrareg.json' + ] - # Check submodules - submodules = module_version.get_submodules() - submodules.sort(key=lambda x: x.path) - assert len(submodules) == 2 - assert [sm.path for sm in submodules] == ['modules/testmodule1', 'modules/testmodule2'] - # Check examples - examples = module_version.get_examples() - examples.sort(key=lambda x: x.path) - assert len(examples) == 2 - assert [example.path for example in examples] == ['examples/testexample1', 'examples/testexample2'] + # Ensure README is present in module version + assert module_version.get_readme_content() == UploadTestModule.TEST_README_CONTENT - # Check attributes from terrareg - assert module_version.description == 'Test unittest description' - assert module_version.owner == 'Test unittest owner' + # Ensure terraform docs output contains variable and output + assert module_version.get_terraform_inputs() == [ + { + 'default': 'test_default_val', + 'description': 'This is a test input', + 'name': 'test_input', + 'required': False, + 'type': 'string' + } + ] + assert module_version.get_terraform_outputs() == [ + { + 'description': 'test output', + 'name': 'test_output' + } + ] + + # Check submodules + submodules = module_version.get_submodules() + submodules.sort(key=lambda x: x.path) + assert len(submodules) == 2 + assert [sm.path for sm in submodules] == ['modules/testmodule1', 'modules/testmodule2'] + + # Check examples + examples = module_version.get_examples() + examples.sort(key=lambda x: x.path) + assert len(examples) == 2 + assert [example.path for example in examples] == ['examples/testexample1', 'examples/testexample2'] - with mock.patch('terrareg.config.Config.ALLOW_MODULE_HOSTING', terrareg.config.ModuleHostingMode.ENFORCE): + # Check attributes from terrareg + assert module_version.description == 'Test unittest description' + assert module_version.owner == 'Test unittest owner' + + with mock.patch('terrareg.config.Config.ALLOW_MODULE_HOSTING', terrareg.config.ModuleHostingMode.ENFORCE): + if archive_git_path: + assert module_version.get_source_download_url(request_domain='localhost', direct_http_request=True) == 'https://localhost:443/v1/terrareg/modules/testprocessupload/git-path/test/1.1.0/source.zip' + else: + assert module_version.get_source_download_url(request_domain='localhost', direct_http_request=True) == 'https://localhost:443/v1/terrareg/modules/testprocessupload/git-path/test/1.1.0/source.zip//subdirectory/in/testprocessupload-git-path-test' + + # Using git path, URL should always contain the sub-path if archive_git_path: - assert module_version.get_source_download_url(request_domain='localhost', direct_http_request=True) == 'https://localhost:443/v1/terrareg/modules/testprocessupload/git-path/test/1.1.0/source.zip' + assert module_version.get_source_download_url(request_domain='localhost', direct_http_request=True) == 'git::ssh://clone-url.com/testprocessupload/git-path-test//subdirectory/in/testprocessupload-git-path-test?ref=1.1.0' else: - assert module_version.get_source_download_url(request_domain='localhost', direct_http_request=True) == 'https://localhost:443/v1/terrareg/modules/testprocessupload/git-path/test/1.1.0/source.zip//subdirectory/in/testprocessupload-git-path-test' - - # Using git path, URL should always contain the sub-path - if archive_git_path: - assert module_version.get_source_download_url(request_domain='localhost', direct_http_request=True) == 'git::ssh://clone-url.com/testprocessupload/git-path-test//subdirectory/in/testprocessupload-git-path-test?ref=1.1.0' - else: - assert module_version.get_source_download_url(request_domain='localhost', direct_http_request=True) == 'git::ssh://clone-url.com/testprocessupload/git-path-test//subdirectory/in/testprocessupload-git-path-test?ref=1.1.0' + assert module_version.get_source_download_url(request_domain='localhost', direct_http_request=True) == 'git::ssh://clone-url.com/testprocessupload/git-path-test//subdirectory/in/testprocessupload-git-path-test?ref=1.1.0' + finally: + module_provider.update_git_provider(None) + module_provider.update_git_path(None) + module_provider.update_archive_git_path(False) def test_uploading_module_with_invalid_terraform(self): """Test uploading a module with invalid terraform.""" From bce2003afcacec154c6e5afefc605e53efe5e02e Mon Sep 17 00:00:00 2001 From: Matthew John Date: Fri, 21 Jun 2024 07:41:21 +0100 Subject: [PATCH 14/20] docs: Update API documentation for module provider settings, adding new archive_git_path parameter Issue #513 --- docs/API.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/API.md b/docs/API.md index 14d64d9e..0e6da2f9 100644 --- a/docs/API.md +++ b/docs/API.md @@ -803,6 +803,7 @@ Handle update to settings. | repo_browse_url_template | json | str | False | `None` | Templated URL for browsing repository. | | git_tag_format | json | str | False | `None` | Module provider git tag format. | | git_path | json | str | False | `None` | Path within git repository that the module exists. | +| archive_git_path | json | boolean | False | `False` | Whether to generate module archives from the git_path directory. Otherwise, archives are generated from the root | | csrf_token | json | str | False | `None` | CSRF token | @@ -839,6 +840,7 @@ Handle update to settings. | repo_browse_url_template | json | str | False | `None` | Templated URL for browsing repository. | | git_tag_format | json | str | False | `None` | Module provider git tag format. | | git_path | json | str | False | `None` | Path within git repository that the module exists. | +| archive_git_path | json | boolean | False | `None` | Whether to generate module archives from the git_path directory. Otherwise, archives are generated from the root | | verified | json | boolean | False | `None` | Whether module provider is marked as verified. | | namespace | json | str | False | `None` | Name of new namespace to move module/module provider to a new namespace | | module | json | str | False | `None` | New name of module | From 665e73ac81773749b6a094e43953644606af09dd Mon Sep 17 00:00:00 2001 From: Matthew John Date: Fri, 21 Jun 2024 07:43:02 +0100 Subject: [PATCH 15/20] chore: Fix typo in comment Issue #513 --- terrareg/server/api/terrareg_module_provider_create.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/terrareg/server/api/terrareg_module_provider_create.py b/terrareg/server/api/terrareg_module_provider_create.py index 953852f6..2dcb32f3 100644 --- a/terrareg/server/api/terrareg_module_provider_create.py +++ b/terrareg/server/api/terrareg_module_provider_create.py @@ -19,7 +19,7 @@ class ApiTerraregModuleProviderCreate(ErrorCatchingResource): request_kwarg_map={'namespace': 'namespace'})] def _post_arg_parser(self): - """Return arg parrser for POST request""" + """Return arg parser for POST request""" parser = reqparse.RequestParser() parser.add_argument( 'git_provider_id', type=str, From dd67e02fd617ea0aaa740514e28e86912cfbd882 Mon Sep 17 00:00:00 2001 From: Matthew John Date: Sat, 22 Jun 2024 07:48:28 +0100 Subject: [PATCH 16/20] test: Update tests for get_source_download_url to ensure that git path is added to source download URLS Issue #513 --- .../terrareg/models/test_module_version.py | 62 +++++++++++-------- 1 file changed, 35 insertions(+), 27 deletions(-) diff --git a/test/integration/terrareg/models/test_module_version.py b/test/integration/terrareg/models/test_module_version.py index d02a3b66..d568fe1b 100644 --- a/test/integration/terrareg/models/test_module_version.py +++ b/test/integration/terrareg/models/test_module_version.py @@ -1505,47 +1505,47 @@ def test_get_source_base_url_with_custom_module_provider_and_module_version_urls with unittest.mock.patch('terrareg.config.Config.ALLOW_CUSTOM_GIT_URL_MODULE_VERSION', False): assert module_version.get_source_base_url() == expected_base_url - @pytest.mark.parametrize('module_name,module_version,git_path,expected_source_download_url,should_prefix_domain, has_git_url', [ + @pytest.mark.parametrize('module_name,module_version,git_path,expected_source_download_url,expected_source_with_archive_git_path,should_prefix_domain,has_git_url', [ # Test no clone URL in any configuration, defaulting to source archive download - ('no-git-provider', '1.0.0', None, '/v1/terrareg/modules/repo_url_tests/no-git-provider/test/1.0.0/source.zip', True, False), + ('no-git-provider', '1.0.0', None, '/v1/terrareg/modules/repo_url_tests/no-git-provider/test/1.0.0/source.zip', '/v1/terrareg/modules/repo_url_tests/no-git-provider/test/1.0.0/source.zip', True, False), # Test clone URL only configured in module version - ('no-git-provider', '1.4.0', None, 'git::ssh://mv-clone-url.com/repo_url_tests/no-git-provider-test?ref=1.4.0', False, True), + ('no-git-provider', '1.4.0', None, 'git::ssh://mv-clone-url.com/repo_url_tests/no-git-provider-test?ref=1.4.0', 'git::ssh://mv-clone-url.com/repo_url_tests/no-git-provider-test?ref=1.4.0', False, True), # Test with git provider configured in module provider - ('git-provider-urls', '1.1.0', None, 'git::ssh://clone-url.com/repo_url_tests/git-provider-urls-test?ref=1.1.0', False, True), + ('git-provider-urls', '1.1.0', None, 'git::ssh://clone-url.com/repo_url_tests/git-provider-urls-test?ref=1.1.0', 'git::ssh://clone-url.com/repo_url_tests/git-provider-urls-test?ref=1.1.0', False, True), # Test with git provider configured in module provider and override in module version - ('git-provider-urls', '1.4.0', None, 'git::ssh://mv-clone-url.com/repo_url_tests/git-provider-urls-test?ref=1.4.0', False, True), + ('git-provider-urls', '1.4.0', None, 'git::ssh://mv-clone-url.com/repo_url_tests/git-provider-urls-test?ref=1.4.0', 'git::ssh://mv-clone-url.com/repo_url_tests/git-provider-urls-test?ref=1.4.0', False, True), # Test with git provider configured in module provider - ('module-provider-urls', '1.2.0', None, 'git::ssh://mp-clone-url.com/repo_url_tests/module-provider-urls-test?ref=1.2.0', False, True), + ('module-provider-urls', '1.2.0', None, 'git::ssh://mp-clone-url.com/repo_url_tests/module-provider-urls-test?ref=1.2.0', 'git::ssh://mp-clone-url.com/repo_url_tests/module-provider-urls-test?ref=1.2.0', False, True), # Test with URls configured in module provider and override in module version - ('module-provider-urls', '1.4.0', None, 'git::ssh://mv-clone-url.com/repo_url_tests/module-provider-urls-test?ref=1.4.0', False, True), + ('module-provider-urls', '1.4.0', None, 'git::ssh://mv-clone-url.com/repo_url_tests/module-provider-urls-test?ref=1.4.0', 'git::ssh://mv-clone-url.com/repo_url_tests/module-provider-urls-test?ref=1.4.0', False, True), # Test with git provider configured in module provider and URLs configured in git provider - ('module-provider-override-git-provider', '1.3.0', None, 'git::ssh://mp-clone-url.com/repo_url_tests/module-provider-override-git-provider-test?ref=1.3.0', False, True), + ('module-provider-override-git-provider', '1.3.0', None, 'git::ssh://mp-clone-url.com/repo_url_tests/module-provider-override-git-provider-test?ref=1.3.0', 'git::ssh://mp-clone-url.com/repo_url_tests/module-provider-override-git-provider-test?ref=1.3.0', False, True), # Test with URls configured in module provider and override in module version - ('module-provider-override-git-provider', '1.4.0', None, 'git::ssh://mv-clone-url.com/repo_url_tests/module-provider-override-git-provider-test?ref=1.4.0', False, True), + ('module-provider-override-git-provider', '1.4.0', None, 'git::ssh://mv-clone-url.com/repo_url_tests/module-provider-override-git-provider-test?ref=1.4.0', 'git::ssh://mv-clone-url.com/repo_url_tests/module-provider-override-git-provider-test?ref=1.4.0', False, True), ## Tests with git_path set for sub-directory of repo # Test no clone URL in any configuration, defaulting to source archive download - ('no-git-provider', '1.0.0', 'sub/directory/of/repo', '/v1/terrareg/modules/repo_url_tests/no-git-provider/test/1.0.0/source.zip', True, False), + ('no-git-provider', '1.0.0', 'sub/directory/of/repo', '/v1/terrareg/modules/repo_url_tests/no-git-provider/test/1.0.0/source.zip//sub/directory/of/repo', '/v1/terrareg/modules/repo_url_tests/no-git-provider/test/1.0.0/source.zip', True, False), # Test clone URL only configured in module version - ('no-git-provider', '1.4.0', 'sub/directory/of/repo', 'git::ssh://mv-clone-url.com/repo_url_tests/no-git-provider-test//sub/directory/of/repo?ref=1.4.0', False, True), + ('no-git-provider', '1.4.0', 'sub/directory/of/repo', 'git::ssh://mv-clone-url.com/repo_url_tests/no-git-provider-test//sub/directory/of/repo?ref=1.4.0', 'git::ssh://mv-clone-url.com/repo_url_tests/no-git-provider-test//sub/directory/of/repo?ref=1.4.0', False, True), # Test with git provider configured in module provider - ('git-provider-urls', '1.1.0', 'sub/directory/of/repo', 'git::ssh://clone-url.com/repo_url_tests/git-provider-urls-test//sub/directory/of/repo?ref=1.1.0', False, True), + ('git-provider-urls', '1.1.0', 'sub/directory/of/repo', 'git::ssh://clone-url.com/repo_url_tests/git-provider-urls-test//sub/directory/of/repo?ref=1.1.0', 'git::ssh://clone-url.com/repo_url_tests/git-provider-urls-test//sub/directory/of/repo?ref=1.1.0', False, True), # Test with git provider configured in module provider and override in module version - ('git-provider-urls', '1.4.0', 'sub/directory/of/repo', 'git::ssh://mv-clone-url.com/repo_url_tests/git-provider-urls-test//sub/directory/of/repo?ref=1.4.0', False, True), + ('git-provider-urls', '1.4.0', 'sub/directory/of/repo', 'git::ssh://mv-clone-url.com/repo_url_tests/git-provider-urls-test//sub/directory/of/repo?ref=1.4.0', 'git::ssh://mv-clone-url.com/repo_url_tests/git-provider-urls-test//sub/directory/of/repo?ref=1.4.0', False, True), # Test with git provider configured in module provider - ('module-provider-urls', '1.2.0', 'sub/directory/of/repo', 'git::ssh://mp-clone-url.com/repo_url_tests/module-provider-urls-test//sub/directory/of/repo?ref=1.2.0', False, True), + ('module-provider-urls', '1.2.0', 'sub/directory/of/repo', 'git::ssh://mp-clone-url.com/repo_url_tests/module-provider-urls-test//sub/directory/of/repo?ref=1.2.0', 'git::ssh://mp-clone-url.com/repo_url_tests/module-provider-urls-test//sub/directory/of/repo?ref=1.2.0', False, True), # Test with URls configured in module provider and override in module version - ('module-provider-urls', '1.4.0', 'sub/directory/of/repo', 'git::ssh://mv-clone-url.com/repo_url_tests/module-provider-urls-test//sub/directory/of/repo?ref=1.4.0', False, True), + ('module-provider-urls', '1.4.0', 'sub/directory/of/repo', 'git::ssh://mv-clone-url.com/repo_url_tests/module-provider-urls-test//sub/directory/of/repo?ref=1.4.0', 'git::ssh://mv-clone-url.com/repo_url_tests/module-provider-urls-test//sub/directory/of/repo?ref=1.4.0', False, True), # Test with git provider configured in module provider and URLs configured in git provider - ('module-provider-override-git-provider', '1.3.0', 'sub/directory/of/repo', 'git::ssh://mp-clone-url.com/repo_url_tests/module-provider-override-git-provider-test//sub/directory/of/repo?ref=1.3.0', False, True), + ('module-provider-override-git-provider', '1.3.0', 'sub/directory/of/repo', 'git::ssh://mp-clone-url.com/repo_url_tests/module-provider-override-git-provider-test//sub/directory/of/repo?ref=1.3.0', 'git::ssh://mp-clone-url.com/repo_url_tests/module-provider-override-git-provider-test//sub/directory/of/repo?ref=1.3.0', False, True), # Test with URls configured in module provider and override in module version - ('module-provider-override-git-provider', '1.4.0', 'sub/directory/of/repo', 'git::ssh://mv-clone-url.com/repo_url_tests/module-provider-override-git-provider-test//sub/directory/of/repo?ref=1.4.0', False, True), + ('module-provider-override-git-provider', '1.4.0', 'sub/directory/of/repo', 'git::ssh://mv-clone-url.com/repo_url_tests/module-provider-override-git-provider-test//sub/directory/of/repo?ref=1.4.0', 'git::ssh://mv-clone-url.com/repo_url_tests/module-provider-override-git-provider-test//sub/directory/of/repo?ref=1.4.0', False, True), ]) @pytest.mark.parametrize('public_url, direct_http_download, expected_url_prefix', [ (None, False, ''), @@ -1564,26 +1564,32 @@ def test_get_source_base_url_with_custom_module_provider_and_module_version_urls terrareg.config.ModuleHostingMode.DISALLOW, terrareg.config.ModuleHostingMode.ENFORCE, ]) - def test_get_source_download_url(self, module_name, module_version, git_path, expected_source_download_url, should_prefix_domain, has_git_url, - public_url, direct_http_download, expected_url_prefix, allow_module_hosting): + @pytest.mark.parametrize('archive_git_path', [ + True, + False + ]) + def test_get_source_download_url(self, module_name, module_version, git_path, expected_source_download_url, expected_source_with_archive_git_path, should_prefix_domain, has_git_url, + public_url, direct_http_download, expected_url_prefix, allow_module_hosting, archive_git_path): """Ensure clone URL matches the expected values.""" namespace = Namespace(name='repo_url_tests') module = Module(namespace=namespace, name=module_name) module_provider = ModuleProvider.get(module=module, name='test') - module_provider.update_git_path(git_path) + module_version_obj = ModuleVersion.get(module_provider=module_provider, version=module_version) + module_version_obj.update_attributes(git_path=git_path, archive_git_path=archive_git_path) try: - assert module_provider is not None - module_version_obj = ModuleVersion.get(module_provider=module_provider, version=module_version) - assert module_version_obj is not None - # When module hosting is enforced, ensure the URL is for the hosted module if allow_module_hosting is terrareg.config.ModuleHostingMode.ENFORCE: - expected_source_download_url = f"/v1/terrareg/modules/repo_url_tests/{module_name}/test/{module_version}/source.zip" - should_prefix_domain = True + expected_source_with_archive_git_path = f"/v1/terrareg/modules/repo_url_tests/{module_name}/test/{module_version}/source.zip" + expected_source_download_url = expected_source_with_archive_git_path + if git_path: + expected_source_download_url += f"//{git_path}" + if direct_http_download: + should_prefix_domain = True if should_prefix_domain: expected_source_download_url = f"{expected_url_prefix}{expected_source_download_url}" + expected_source_with_archive_git_path = f"{expected_url_prefix}{expected_source_with_archive_git_path}" with unittest.mock.patch('terrareg.config.Config.PUBLIC_URL', public_url), \ unittest.mock.patch('terrareg.config.Config.ALLOW_MODULE_HOSTING', allow_module_hosting): @@ -1593,11 +1599,13 @@ def test_get_source_download_url(self, module_name, module_version, git_path, ex if not has_git_url and allow_module_hosting is terrareg.config.ModuleHostingMode.DISALLOW: with pytest.raises(terrareg.errors.NoModuleDownloadMethodConfiguredError): module_version_obj.get_source_download_url(request_domain="localhost", direct_http_request=direct_http_download) + elif archive_git_path: + assert module_version_obj.get_source_download_url(request_domain="localhost", direct_http_request=direct_http_download) == expected_source_with_archive_git_path else: assert module_version_obj.get_source_download_url(request_domain="localhost", direct_http_request=direct_http_download) == expected_source_download_url finally: - module_provider.update_git_path(None) + module_version_obj.update_attributes(git_path=None, archive_git_path=False) @pytest.mark.parametrize('git_sha,module_version_use_git_commit,expected_source_download_url', [ # Test without git sha and use_git_commit not enabled From 13fb959d8de1aaaeab739e89d298ca7d3532a447 Mon Sep 17 00:00:00 2001 From: Matthew John Date: Sat, 22 Jun 2024 10:01:34 +0100 Subject: [PATCH 17/20] test: Update module version tests to update git path on module version object, rather than module provider Issue #513 --- .../terrareg/models/test_module_version.py | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/test/integration/terrareg/models/test_module_version.py b/test/integration/terrareg/models/test_module_version.py index d568fe1b..bca53a4b 100644 --- a/test/integration/terrareg/models/test_module_version.py +++ b/test/integration/terrareg/models/test_module_version.py @@ -1101,18 +1101,17 @@ def test_get_source_browse_url(self, module_name, module_version, git_path, path module = Module(namespace=namespace, name=module_name) module_provider = ModuleProvider.get(module=module, name='test') assert module_provider is not None + module_version = ModuleVersion.get(module_provider=module_provider, version=module_version) + assert module_version is not None - module_provider.update_git_path(git_path) + module_version.update_attributes(git_path=git_path) try: - module_version = ModuleVersion.get(module_provider=module_provider, version=module_version) - assert module_version is not None - kwargs = {'path': path} if path else {} assert module_version.get_source_browse_url(**kwargs) == expected_browse_url finally: - module_provider.update_git_path(None) + module_version.update_attributes(git_path=None) @pytest.mark.parametrize('module_name,module_version,path,expected_browse_url', [ # Test no browse URL in any configuration @@ -1664,13 +1663,12 @@ def test_get_source_download_url_presigned(self, module_name, module_version, gi namespace = Namespace(name='repo_url_tests') module = Module(namespace=namespace, name=module_name) module_provider = ModuleProvider.get(module=module, name='test') - module_provider.update_git_path(git_path) + assert module_provider is not None + module_version = ModuleVersion.get(module_provider=module_provider, version=module_version) + assert module_version is not None + module_version.update_attributes(git_path=git_path) try: - assert module_provider is not None - module_version = ModuleVersion.get(module_provider=module_provider, version=module_version) - assert module_version is not None - mock_generate_presigned_key = unittest.mock.MagicMock(return_value='unittest-presign-key') with unittest.mock.patch('terrareg.presigned_url.TerraformSourcePresignedUrl.generate_presigned_key', mock_generate_presigned_key), \ unittest.mock.patch('terrareg.config.Config.ALLOW_UNAUTHENTICATED_ACCESS', allow_unauthenticated_access): @@ -1691,7 +1689,7 @@ def test_get_source_download_url_presigned(self, module_name, module_version, gi mock_generate_presigned_key.assert_not_called() finally: - module_provider.update_git_path(None) + module_version.update_attributes(git_path=None) @pytest.mark.parametrize('published,beta,is_latest_version,expected_value', [ # Latest published non-beta From 483b0020450c4e6f59e6876d17d1b2783cd0df53 Mon Sep 17 00:00:00 2001 From: Matthew John Date: Fri, 5 Jul 2024 07:54:11 +0100 Subject: [PATCH 18/20] test: Update tests for module version model to ensure git path is configured from git provider Issue #513 --- .../terrareg/models/test_module_version.py | 37 ++++++++++++++++--- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/test/integration/terrareg/models/test_module_version.py b/test/integration/terrareg/models/test_module_version.py index bca53a4b..f9aed678 100644 --- a/test/integration/terrareg/models/test_module_version.py +++ b/test/integration/terrareg/models/test_module_version.py @@ -15,6 +15,7 @@ import terrareg.config import terrareg.errors from test.integration.terrareg import TerraregIntegrationTest +from test.integration.terrareg.module_extractor import UploadTestModule class TestModuleVersion(TerraregIntegrationTest): @@ -1018,12 +1019,38 @@ def test_get_readme_html(self, readme_content, example_analaytics_token, expecte def test_git_path(self): """Test git_path property""" - # Ensure the git_path from the module provider is returned - with unittest.mock.patch('terrareg.models.ModuleProvider.git_path', 'unittest-git-path'): - module_provider = ModuleProvider.get(Module(Namespace('moduledetails'), 'git-path'), 'provider') - module_version = ModuleVersion.get(module_provider, '1.0.0') - assert module_version.git_path == 'unittest-git-path' + # Create module provider with git path + module = Module(Namespace('moduledetails'), 'git-path') + module_provider = ModuleProvider.create(module=module, name='testgitpath') + module_version = None + try: + module_provider.update_git_path("/unit-test-git-path") + + # Create module version. + module_version = ModuleVersion(module_provider=module_provider, version='1.0.0') + module_version.prepare_module() + test_upload = UploadTestModule() + with test_upload as zip_file: + with test_upload as upload_dir: + os.mkdir(os.path.join(upload_dir, "unit-test-git-path")) + with open(os.path.join(upload_dir, "unit-test-git-path", "test.tf"), "w") as fh: + fh.write(test_upload.VALID_MAIN_TF_FILE) + UploadTestModule.upload_module_version(module_version=module_version, zip_file=zip_file) + + # Ensure git path is copied to version + assert module_version.git_path == "unit-test-git-path" + + # Update git path in module provider and ensure the git path of the module version is + # still the original + module_provider.update_git_path("new/unittest/git/path") + module_version = ModuleVersion.get(module_provider=module_provider, version='1.0.0') + assert module_version.git_path == "unit-test-git-path" + + finally: + if module_version: + module_version.delete() + module_provider.delete() @pytest.mark.parametrize('module_name,module_version,git_path,path,expected_browse_url', [ # Test no browse URL in any configuration From 506ab635a71bbfd70ba35c6ee375c05626cac8e9 Mon Sep 17 00:00:00 2001 From: Matthew John Date: Fri, 5 Jul 2024 07:54:32 +0100 Subject: [PATCH 19/20] test: Update submodule tests to set git path on module version Issue #513 --- test/integration/terrareg/models/test_base_submodule.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/terrareg/models/test_base_submodule.py b/test/integration/terrareg/models/test_base_submodule.py index c64df6e7..33d6765f 100644 --- a/test/integration/terrareg/models/test_base_submodule.py +++ b/test/integration/terrareg/models/test_base_submodule.py @@ -31,8 +31,8 @@ class CommonBaseSubmodule(TerraregIntegrationTest): ]) def test_git_path(self, provider_git_path, module_path, expected_path): provider = ModuleProvider.get(Module(Namespace('moduledetails'), 'git-path'), 'provider') - provider.update_git_path(provider_git_path) version = ModuleVersion.get(provider, '1.0.0') + version.update_attributes(git_path=provider_git_path) submodule = self.SUBMODULE_CLASS.create(version, module_path) try: From 99c84a0d64319bc7de4338c169da3041289bb323 Mon Sep 17 00:00:00 2001 From: Matthew John Date: Sat, 6 Jul 2024 09:18:25 +0100 Subject: [PATCH 20/20] db: Update DB migeration for git path and archive_git_path to include new audit action for modifying archive_git_path Issue #513 --- ..._add_archive_git_path_column_to_module_.py | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/terrareg/alembic/versions/f9a80ea383cc_add_archive_git_path_column_to_module_.py b/terrareg/alembic/versions/f9a80ea383cc_add_archive_git_path_column_to_module_.py index b03be9dc..3915caa9 100644 --- a/terrareg/alembic/versions/f9a80ea383cc_add_archive_git_path_column_to_module_.py +++ b/terrareg/alembic/versions/f9a80ea383cc_add_archive_git_path_column_to_module_.py @@ -38,11 +38,57 @@ def upgrade(): module_provider_id=module_provider_id, ) + if op.get_bind().engine.name == 'mysql': + op.alter_column('audit_history', 'action', + existing_type=sa.Enum( + 'NAMESPACE_CREATE', 'NAMESPACE_MODIFY_NAME', 'NAMESPACE_MODIFY_DISPLAY_NAME', 'MODULE_PROVIDER_CREATE', 'MODULE_PROVIDER_DELETE', 'MODULE_PROVIDER_UPDATE_GIT_TAG_FORMAT', + 'MODULE_PROVIDER_UPDATE_GIT_PROVIDER', 'MODULE_PROVIDER_UPDATE_GIT_PATH', 'MODULE_PROVIDER_UPDATE_GIT_CUSTOM_BASE_URL', + 'MODULE_PROVIDER_UPDATE_GIT_CUSTOM_CLONE_URL', 'MODULE_PROVIDER_UPDATE_GIT_CUSTOM_BROWSE_URL', 'MODULE_PROVIDER_UPDATE_VERIFIED', + 'MODULE_VERSION_INDEX', 'MODULE_VERSION_PUBLISH', 'MODULE_VERSION_DELETE', 'USER_GROUP_CREATE', 'USER_GROUP_DELETE', 'USER_GROUP_NAMESPACE_PERMISSION_ADD', + 'USER_GROUP_NAMESPACE_PERMISSION_MODIFY', 'USER_GROUP_NAMESPACE_PERMISSION_DELETE', 'USER_LOGIN', + 'MODULE_PROVIDER_UPDATE_NAMESPACE', 'MODULE_PROVIDER_UPDATE_MODULE_NAME', 'MODULE_PROVIDER_UPDATE_PROVIDER_NAME', 'NAMESPACE_DELETE', 'MODULE_PROVIDER_REDIRECT_DELETE', + 'GPG_KEY_CREATE', 'GPG_KEY_DELETE', 'PROVIDER_CREATE', 'PROVIDER_DELETE', 'PROVIDER_VERSION_INDEX', 'PROVIDER_VERSION_DELETE', 'REPOSITORY_CREATE', 'REPOSITORY_UPDATE', 'REPOSITORY_DELETE', + name='auditaction'), + type_=sa.Enum( + 'NAMESPACE_CREATE', 'NAMESPACE_MODIFY_NAME', 'NAMESPACE_MODIFY_DISPLAY_NAME', 'MODULE_PROVIDER_CREATE', 'MODULE_PROVIDER_DELETE', 'MODULE_PROVIDER_UPDATE_GIT_TAG_FORMAT', + 'MODULE_PROVIDER_UPDATE_GIT_PROVIDER', 'MODULE_PROVIDER_UPDATE_GIT_PATH', 'MODULE_PROVIDER_UPDATE_GIT_CUSTOM_BASE_URL', + 'MODULE_PROVIDER_UPDATE_GIT_CUSTOM_CLONE_URL', 'MODULE_PROVIDER_UPDATE_GIT_CUSTOM_BROWSE_URL', 'MODULE_PROVIDER_UPDATE_VERIFIED', + 'MODULE_VERSION_INDEX', 'MODULE_VERSION_PUBLISH', 'MODULE_VERSION_DELETE', 'USER_GROUP_CREATE', 'USER_GROUP_DELETE', 'USER_GROUP_NAMESPACE_PERMISSION_ADD', + 'USER_GROUP_NAMESPACE_PERMISSION_MODIFY', 'USER_GROUP_NAMESPACE_PERMISSION_DELETE', 'USER_LOGIN', + 'MODULE_PROVIDER_UPDATE_NAMESPACE', 'MODULE_PROVIDER_UPDATE_MODULE_NAME', 'MODULE_PROVIDER_UPDATE_PROVIDER_NAME', 'NAMESPACE_DELETE', 'MODULE_PROVIDER_REDIRECT_DELETE', + 'GPG_KEY_CREATE', 'GPG_KEY_DELETE', 'PROVIDER_CREATE', 'PROVIDER_DELETE', 'PROVIDER_VERSION_INDEX', 'PROVIDER_VERSION_DELETE', 'REPOSITORY_CREATE', 'REPOSITORY_UPDATE', 'REPOSITORY_DELETE', + 'MODULE_PROVIDER_UPDATE_ARCHIVE_GIT_PATH', + name='auditaction'), + nullable=False) # ### end Alembic commands ### def downgrade(): # ### commands auto generated by Alembic - please adjust! ### + + if op.get_bind().engine.name == 'mysql': + op.alter_column('audit_history', 'action', + existing_type=sa.Enum( + 'NAMESPACE_CREATE', 'NAMESPACE_MODIFY_NAME', 'NAMESPACE_MODIFY_DISPLAY_NAME', 'MODULE_PROVIDER_CREATE', 'MODULE_PROVIDER_DELETE', 'MODULE_PROVIDER_UPDATE_GIT_TAG_FORMAT', + 'MODULE_PROVIDER_UPDATE_GIT_PROVIDER', 'MODULE_PROVIDER_UPDATE_GIT_PATH', 'MODULE_PROVIDER_UPDATE_GIT_CUSTOM_BASE_URL', + 'MODULE_PROVIDER_UPDATE_GIT_CUSTOM_CLONE_URL', 'MODULE_PROVIDER_UPDATE_GIT_CUSTOM_BROWSE_URL', 'MODULE_PROVIDER_UPDATE_VERIFIED', + 'MODULE_VERSION_INDEX', 'MODULE_VERSION_PUBLISH', 'MODULE_VERSION_DELETE', 'USER_GROUP_CREATE', 'USER_GROUP_DELETE', 'USER_GROUP_NAMESPACE_PERMISSION_ADD', + 'USER_GROUP_NAMESPACE_PERMISSION_MODIFY', 'USER_GROUP_NAMESPACE_PERMISSION_DELETE', 'USER_LOGIN', + 'MODULE_PROVIDER_UPDATE_NAMESPACE', 'MODULE_PROVIDER_UPDATE_MODULE_NAME', 'MODULE_PROVIDER_UPDATE_PROVIDER_NAME', 'NAMESPACE_DELETE', 'MODULE_PROVIDER_REDIRECT_DELETE', + 'GPG_KEY_CREATE', 'GPG_KEY_DELETE', 'PROVIDER_CREATE', 'PROVIDER_DELETE', 'PROVIDER_VERSION_INDEX', 'PROVIDER_VERSION_DELETE', 'REPOSITORY_CREATE', 'REPOSITORY_UPDATE', 'REPOSITORY_DELETE', + 'MODULE_PROVIDER_UPDATE_ARCHIVE_GIT_PATH', + name='auditaction'), + type_=sa.Enum( + 'NAMESPACE_CREATE', 'NAMESPACE_MODIFY_NAME', 'NAMESPACE_MODIFY_DISPLAY_NAME', 'MODULE_PROVIDER_CREATE', 'MODULE_PROVIDER_DELETE', 'MODULE_PROVIDER_UPDATE_GIT_TAG_FORMAT', + 'MODULE_PROVIDER_UPDATE_GIT_PROVIDER', 'MODULE_PROVIDER_UPDATE_GIT_PATH', 'MODULE_PROVIDER_UPDATE_GIT_CUSTOM_BASE_URL', + 'MODULE_PROVIDER_UPDATE_GIT_CUSTOM_CLONE_URL', 'MODULE_PROVIDER_UPDATE_GIT_CUSTOM_BROWSE_URL', 'MODULE_PROVIDER_UPDATE_VERIFIED', + 'MODULE_VERSION_INDEX', 'MODULE_VERSION_PUBLISH', 'MODULE_VERSION_DELETE', 'USER_GROUP_CREATE', 'USER_GROUP_DELETE', 'USER_GROUP_NAMESPACE_PERMISSION_ADD', + 'USER_GROUP_NAMESPACE_PERMISSION_MODIFY', 'USER_GROUP_NAMESPACE_PERMISSION_DELETE', 'USER_LOGIN', + 'MODULE_PROVIDER_UPDATE_NAMESPACE', 'MODULE_PROVIDER_UPDATE_MODULE_NAME', 'MODULE_PROVIDER_UPDATE_PROVIDER_NAME', 'NAMESPACE_DELETE', 'MODULE_PROVIDER_REDIRECT_DELETE', + 'GPG_KEY_CREATE', 'GPG_KEY_DELETE', 'PROVIDER_CREATE', 'PROVIDER_DELETE', 'PROVIDER_VERSION_INDEX', 'PROVIDER_VERSION_DELETE', 'REPOSITORY_CREATE', 'REPOSITORY_UPDATE', 'REPOSITORY_DELETE', + name='auditaction'), + nullable=False) + with op.batch_alter_table('module_provider', schema=None) as batch_op: batch_op.drop_column('archive_git_path') with op.batch_alter_table('module_version', schema=None) as batch_op: