Skip to content

Add new ssh key argument and don't check magic number in old versions #234

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
pablogsal wants to merge 4 commits into from
Closed

Add new ssh key argument and don't check magic number in old versions #234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6895e82
Add new ssh key argument and don't check magic number in old versions
pablogsal Apr 8, 2025
cb21016
Add a new --security-release flag
pablogsal Apr 8, 2025
9ff1c99
Format with Black to fix lint
hugovk Apr 8, 2025
f7e1b8d
Fix test
hugovk Apr 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions release.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,19 @@ def get(self, key: Literal["auth_info"], default: str | None = None) -> str: ...
@overload
def get(self, key: Literal["ssh_user"], default: str | None = None) -> str: ...

@overload
def get(
self, key: Literal["ssh_key"], default: str | None = None
) -> str | None: ...

@overload
def get(self, key: Literal["sign_gpg"], default: bool | None = None) -> bool: ...

@overload
def get(
self, key: Literal["security_release"], default: bool | None = None
) -> bool: ...

@overload
def get(self, key: Literal["release"], default: Tag | None = None) -> Tag: ...

Expand All @@ -84,9 +94,15 @@ def __getitem__(self, key: Literal["auth_info"]) -> str: ...
@overload
def __getitem__(self, key: Literal["ssh_user"]) -> str: ...

@overload
def __getitem__(self, key: Literal["ssh_key"]) -> str | None: ...

@overload
def __getitem__(self, key: Literal["sign_gpg"]) -> bool: ...

@overload
def __getitem__(self, key: Literal["security_release"]) -> bool: ...

@overload
def __getitem__(self, key: Literal["release"]) -> Tag: ...

Expand All @@ -110,9 +126,15 @@ def __setitem__(self, key: Literal["auth_info"], value: str) -> None: ...
@overload
def __setitem__(self, key: Literal["ssh_user"], value: str) -> None: ...

@overload
def __setitem__(self, key: Literal["ssh_key"], value: str | None) -> None: ...

@overload
def __setitem__(self, key: Literal["sign_gpg"], value: bool) -> None: ...

@overload
def __setitem__(self, key: Literal["security_release"], value: bool) -> None: ...

@overload
def __setitem__(self, key: Literal["release"], value: Tag) -> None: ...

Expand Down
91 changes: 74 additions & 17 deletions run_release.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,8 @@ def __init__(
api_key: str,
ssh_user: str,
sign_gpg: bool,
ssh_key: str | None = None,
security_release: bool = False,
first_state: Task | None = None,
) -> None:
self.tasks = tasks
Expand All @@ -215,8 +217,12 @@ def __init__(
self.db["auth_info"] = api_key
if not self.db.get("ssh_user"):
self.db["ssh_user"] = ssh_user
if not self.db.get("ssh_key"):
self.db["ssh_key"] = ssh_key
if not self.db.get("sign_gpg"):
self.db["sign_gpg"] = sign_gpg
if not self.db.get("security_release"):
self.db["security_release"] = security_release

if not self.db.get("release"):
self.db["release"] = release_tag
Expand All @@ -227,8 +233,10 @@ def __init__(
print(f"- Normalized release tag: {release_tag.normalized()}")
print(f"- Git repo: {self.db['git_repo']}")
print(f"- SSH username: {self.db['ssh_user']}")
print(f"- SSH key: {self.db['ssh_key'] or 'Default'}")
print(f"- python.org API key: {self.db['auth_info']}")
print(f"- Sign with GPG: {self.db['sign_gpg']}")
print(f"- Security release: {self.db['security_release']}")
print()

def checkpoint(self) -> None:
Expand Down Expand Up @@ -313,17 +321,23 @@ def check_ssh_connection(db: ReleaseShelf) -> None:
client = paramiko.SSHClient()
client.load_system_host_keys()
client.set_missing_host_key_policy(paramiko.WarningPolicy)
client.connect(DOWNLOADS_SERVER, port=22, username=db["ssh_user"])
client.connect(
DOWNLOADS_SERVER, port=22, username=db["ssh_user"], key_filename=db["ssh_key"]
)
client.exec_command("pwd")
client.connect(DOCS_SERVER, port=22, username=db["ssh_user"])
client.connect(
DOCS_SERVER, port=22, username=db["ssh_user"], key_filename=db["ssh_key"]
)
client.exec_command("pwd")


def check_sigstore_client(db: ReleaseShelf) -> None:
client = paramiko.SSHClient()
client.load_system_host_keys()
client.set_missing_host_key_policy(paramiko.WarningPolicy)
client.connect(DOWNLOADS_SERVER, port=22, username=db["ssh_user"])
client.connect(
DOWNLOADS_SERVER, port=22, username=db["ssh_user"], key_filename=db["ssh_key"]
)
_, stdout, _ = client.exec_command("python3 -m sigstore --version")
sigstore_version = stdout.read(1000).decode()
sigstore_vermatch = re.match("^sigstore ([0-9.]+)", sigstore_version)
Expand Down Expand Up @@ -398,6 +412,9 @@ def check_cpython_repo_is_clean(db: ReleaseShelf) -> None:

def check_magic_number(db: ReleaseShelf) -> None:
release_tag = db["release"]
if release_tag.major == 3 and release_tag.minor <= 13:
Copy link
Copy Markdown
Member

@hugovk hugovk Apr 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to avoid hardcoding this, as it's another thing that needs adding to a longish list when a new version comes around.

Perhaps we can fetch the newest version from https://github.com/python/devguide/blob/main/include/release-cycle.json (and later from python/peps#4331)?

This can also be a followup PR.

Copy link
Copy Markdown
Member Author

@pablogsal pablogsal Apr 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the first version that needed this feature. How fetching the newer version would be cleaner ?

Copy link
Copy Markdown
Member

@AA-Turner AA-Turner Apr 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it always only the current feature release that this check will apply for? If so, perhaps expanding the security Boolean to a branch status enum would be better, as then we could test if branch-status == feature here.

A

Copy link
Copy Markdown
Member Author

@pablogsal pablogsal Apr 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this particular check applies to all releases that are after we changed that file, that is, anything above or including 3.13

Copy link
Copy Markdown
Member

@hugovk hugovk Apr 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, if the limit is always 3.13 and we don't need to change it, then a hardcoded version makes sense here.

Let's use a tuple so we don't need to worry about Python 4 :)

Suggested change
if release_tag.major == 3 and release_tag.minor <= 13:
if not release_tag.as_tuple() >= (3, 14):

Copy link
Copy Markdown
Member

@merwok merwok Nov 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be written using <= ?

Copy link
Copy Markdown
Member

@hugovk hugovk Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, or rather as < (3, 13). I'll open a new PR.

return

if release_tag.is_final or release_tag.is_release_candidate:

def out(msg: str) -> None:
Expand Down Expand Up @@ -623,7 +640,7 @@ def sign_source_artifacts(db: ReleaseShelf) -> None:

subprocess.check_call(
[
"python3",
sys.executable,
"-m",
"sigstore",
"sign",
Expand Down Expand Up @@ -692,7 +709,7 @@ def upload_files_to_server(db: ReleaseShelf, server: str) -> None:
client = paramiko.SSHClient()
client.load_system_host_keys()
client.set_missing_host_key_policy(paramiko.WarningPolicy)
client.connect(server, port=22, username=db["ssh_user"])
client.connect(server, port=22, username=db["ssh_user"], key_filename=db["ssh_key"])
transport = client.get_transport()
assert transport is not None, f"SSH transport to {server} is None"

Expand Down Expand Up @@ -737,7 +754,9 @@ def place_files_in_download_folder(db: ReleaseShelf) -> None:
client = paramiko.SSHClient()
client.load_system_host_keys()
client.set_missing_host_key_policy(paramiko.WarningPolicy)
client.connect(DOWNLOADS_SERVER, port=22, username=db["ssh_user"])
client.connect(
DOWNLOADS_SERVER, port=22, username=db["ssh_user"], key_filename=db["ssh_key"]
)
transport = client.get_transport()
assert transport is not None, f"SSH transport to {DOWNLOADS_SERVER} is None"

Expand Down Expand Up @@ -788,7 +807,9 @@ def unpack_docs_in_the_docs_server(db: ReleaseShelf) -> None:
client = paramiko.SSHClient()
client.load_system_host_keys()
client.set_missing_host_key_policy(paramiko.WarningPolicy)
client.connect(DOCS_SERVER, port=22, username=db["ssh_user"])
client.connect(
DOCS_SERVER, port=22, username=db["ssh_user"], key_filename=db["ssh_key"]
)
transport = client.get_transport()
assert transport is not None, f"SSH transport to {DOCS_SERVER} is None"

Expand Down Expand Up @@ -905,7 +926,9 @@ def wait_until_all_files_are_in_folder(db: ReleaseShelf) -> None:
client = paramiko.SSHClient()
client.load_system_host_keys()
client.set_missing_host_key_policy(paramiko.WarningPolicy)
client.connect(DOWNLOADS_SERVER, port=22, username=db["ssh_user"])
client.connect(
DOWNLOADS_SERVER, port=22, username=db["ssh_user"], key_filename=db["ssh_key"]
)
ftp_client = client.open_sftp()

destination = f"/srv/www.python.org/ftp/python/{db['release'].normalized()}"
Expand All @@ -923,18 +946,34 @@ def wait_until_all_files_are_in_folder(db: ReleaseShelf) -> None:
are_windows_files_there = f"python-{release}.exe" in all_files
are_macos_files_there = f"python-{release}-macos11.pkg" in all_files
are_linux_files_there = f"Python-{release}.tgz" in all_files
are_all_files_there = (
are_linux_files_there and are_windows_files_there and are_macos_files_there
)

if db["security_release"]:
# For security releases, only check Linux files
are_all_files_there = are_linux_files_there
else:
# For regular releases, check all platforms
are_all_files_there = (
are_linux_files_there
and are_windows_files_there
and are_macos_files_there
)

if not are_all_files_there:
linux_tick = "✅" if are_linux_files_there else "❌"
windows_tick = "✅" if are_windows_files_there else "❌"
macos_tick = "✅" if are_macos_files_there else "❌"
print(
f"\rWaiting for files: Linux {linux_tick} Windows {windows_tick} Mac {macos_tick} ",
flush=True,
end="",
)
if db["security_release"]:
print(
f"\rWaiting for files: Linux {linux_tick} (security release mode - only checking Linux) ",
flush=True,
end="",
)
else:
print(
f"\rWaiting for files: Linux {linux_tick} Windows {windows_tick} Mac {macos_tick} ",
flush=True,
end="",
)
time.sleep(1)
print()

Expand All @@ -943,7 +982,9 @@ def run_add_to_python_dot_org(db: ReleaseShelf) -> None:
client = paramiko.SSHClient()
client.load_system_host_keys()
client.set_missing_host_key_policy(paramiko.WarningPolicy)
client.connect(DOWNLOADS_SERVER, port=22, username=db["ssh_user"])
client.connect(
DOWNLOADS_SERVER, port=22, username=db["ssh_user"], key_filename=db["ssh_key"]
)
transport = client.get_transport()
assert transport is not None, f"SSH transport to {DOWNLOADS_SERVER} is None"

Expand Down Expand Up @@ -1259,6 +1300,20 @@ def _api_key(api_key: str) -> str:
help="Username to be used when authenticating via ssh",
type=str,
)
parser.add_argument(
"--ssh-key",
dest="ssh_key",
default=None,
help="Path to the SSH key file to use for authentication",
type=str,
)
parser.add_argument(
"--security-release",
dest="security_release",
action="store_true",
default=False,
help="Indicate this is a security release (only checks for Linux files)",
)
Comment on lines +1310 to +1316
Copy link
Copy Markdown
Member

@hugovk hugovk Apr 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And similarly, we could check the status in the JSON file.

Copy link
Copy Markdown
Member

@hugovk hugovk Apr 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's a PR to check the status from the devguide, so the RM doesn't have to remember to pass --security each time:

pablogsal#1

Merging that will update this PR.

args = parser.parse_args()

auth_key = args.auth_key or os.getenv("AUTH_INFO")
Expand Down Expand Up @@ -1353,6 +1408,8 @@ def _api_key(api_key: str) -> str:
api_key=auth_key,
ssh_user=args.ssh_user,
sign_gpg=not no_gpg,
ssh_key=args.ssh_key,
security_release=args.security_release,
tasks=tasks,
)
automata.run()
Expand Down
2 changes: 1 addition & 1 deletion tests/test_run_release.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def test_invalid_extract_github_owner() -> None:

def test_check_magic_number() -> None:
db = {
"release": Tag("3.13.0rc1"),
"release": Tag("3.14.0rc1"),
"git_repo": str(Path(__file__).parent / "magicdata"),
}
with pytest.raises(
Expand Down