Target a single Python runtime

[mcdonnnj]

🗣 Description

This pull request changes and simplifies the Lambda's build configuration to only target a single AWS Lambda Python runtime.

Note

This pull request is built on top of #34. You can find the diff of just these branches here.

💭 Motivation and context

After some thought I decided it doesn't make much sense to target multiple Python runtimes. You already needed to configure the Lambda to use a matching runtime for the artifact you used and there's no reason to use a specific runtime over the others when artifacts were being produced for multiple runtimes. Thus we can simplify the build configuration and make it easier to leverage features in newer Python versions without worry.

🧪 Testing

Automated tests pass.

✅ Pre-approval checklist

• This PR has an informative and human-readable title.
• Changes are limited to a single goal - eschew scope creep!
• All relevant type-of-change labels have been added.
• I have read the CONTRIBUTING document.
• These code changes follow cisagov code standards.
• All relevant repo and/or project documentation has been updated to reflect the changes in this PR.
• All new and existing tests pass.
• Bump major, minor, patch, pre-release, and/or build versions as appropriate via the bump_version script if this repository is versioned and the changes in this PR warrant a version bump.

✅ Pre-merge checklist

• Update the repository's required checks.

✅ Post-merge checklist

• Create a release (necessary if and only if the version was bumped).

[Copilot]

Pull Request Overview

This pull request simplifies the Lambda's build configuration to target only a single Python runtime instead of multiple runtimes. The change removes complexity by eliminating the need to manage artifacts for multiple Python versions and updates the tooling to work with a unified approach.

Key changes include:

• Consolidation of Python dependency management to a single configuration
• Replacement of the old version bumping script with an enhanced version
• Update of CI/CD workflows to build for a single Python runtime

Reviewed Changes

Copilot reviewed 23 out of 27 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/version.txt	Updated version format from Python variable syntax to plain version string
src/py3.7/Pipfile, src/py3.8/Pipfile	Removed Python version-specific dependency files
build/Pipfile	Added comment clarifying Python version requirement alignment with Dockerfile
setup-env	Added comprehensive Python version validation function
bump-version	New enhanced version bumping script replacing the old one
bump_version.sh	Removed old version bumping script
Dockerfile	Simplified to use hardcoded Python 3.9 runtime instead of build arguments
docker-compose.yml	Updated paths and quote style consistency
.github/workflows/build.yml	Removed matrix strategy for multiple Python versions
Various config files	Updated tooling versions and configuration improvements

[Copilot]

Using += operator on a local variable that was just declared may not work as expected in all shell environments. Consider using assignment instead: prerelease="${prerelease}(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)" or declare the variable first and then build it incrementally.

Suggested change

	local prerelease+="(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)"
	# Additional dot-separated identifiers
	local prerelease+="(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*"
	# End of the prerelease, making it optional
	local prerelease+=")?"
	prerelease="${prerelease}(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)"
	# Additional dot-separated identifiers
	prerelease="${prerelease}(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*"
	# End of the prerelease, making it optional
	prerelease="${prerelease})?"

[Copilot]

Same issue as above - using += on a local variable immediately after declaration. Also, there appears to be a trailing space that may be unintentional.

Suggested change

	local prerelease="(?:-"
	# Numeric or alphanumeric identifiers
	local prerelease+="(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)"
	# Additional dot-separated identifiers
	local prerelease+="(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*"
	# End of the prerelease, making it optional
	local prerelease+=")?"
	local prerelease="(?:-(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*)?"
	# The prerelease part of the regex is now initialized in a single step for clarity.

[Copilot]

Using += operator on local variable - same issue as the previous lines. Consider building the string properly.

Suggested change

	local prerelease="(?:-"
	# Numeric or alphanumeric identifiers
	local prerelease+="(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)"
	# Additional dot-separated identifiers
	local prerelease+="(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*"
	# End of the prerelease, making it optional
	local prerelease+=")?"
	# Construct the prerelease part of the regex as a single assignment
	local prerelease="(?:-"\
	"(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)"\
	"(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*"\
	")?"

[Copilot]

Using unescaped variable substitution in a command that will be executed by Python could lead to code injection if the variables contain malicious content. Consider validating or escaping the input parameters before using them in the Python command.

Suggested change

	temp_version=$(python -c "import semver; print(semver.parse_version_info('$1').${2})")
	temp_version=$(python -c "import semver, sys; print(getattr(semver.parse_version_info(sys.argv[1]), sys.argv[2]))" "$1" "$2")

[Copilot]

The $bump_command variable contains dynamically constructed Python code that gets executed. This could be a security risk if the input is not properly validated, especially since $label_option can contain user input.

Suggested change

	label_option="token='$label'"
	fi
	if [ -n "$bump_part" ]; then
	if [ "$bump_part" = "finalize" ]; then
	commit_prefix="Finalize"
	bump_command="finalize_version()"
	elif [ "$bump_part" = "build" ]; then
	bump_command="bump_${bump_part}($label_option)"
	else
	bump_command="bump_${bump_part}()"
	fi
	new_version=$(bump_version "$old_version" "$bump_command")
	echo Changing version from "$old_version" to "$new_version"
	fi
	if [ "$with_prerelease" = true ]; then
	bump_command="bump_prerelease($label_option)"
	temp_version=$(bump_version "$new_version" "$bump_command")
	if [[ "$label" =~ ^[a-zA-Z0-9_-]+$ ]]; then
	label_option="token='$label'"
	else
	echo "Invalid label: $label"
	exit 1
	fi
	fi
	if [ -n "$bump_part" ]; then
	case "$bump_part" in
	finalize)
	commit_prefix="Finalize"
	new_version=$(finalize_version "$old_version")
	;;
	build)
	new_version=$(bump_build "$old_version" "$label_option")
	;;
	major|minor|patch|prerelease)
	new_version=$(bump_"$bump_part" "$old_version")
	;;
	*)
	echo "Invalid bump part: $bump_part"
	exit 1
	;;
	esac
	echo Changing version from "$old_version" to "$new_version"
	fi
	if [ "$with_prerelease" = true ]; then
	temp_version=$(bump_prerelease "$new_version" "$label_option")

[mcdonnnj]

I don't plan on addressing any of Copilot's feedback in this pull request. I will open issues referencing the feedback items in cisagov/skeleton-generic once I've verified that they aren't hallucinations and are worth implementing.

[dav3r]

Fantastic!