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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

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 @@
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 @@
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

Check warning on line 221 in run_release.py

View check run for this annotation

Codecov / codecov/patch

run_release.py#L220-L221 

Added lines #L220 - L221 were not covered by tests
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

Check warning on line 225 in run_release.py

View check run for this annotation

Codecov / codecov/patch

run_release.py#L224-L225 

Added lines #L224 - L225 were not covered by tests

if not self.db.get("release"):
self.db["release"] = release_tag
Expand All @@ -227,8 +233,10 @@
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'}")

Check warning on line 236 in run_release.py

View check run for this annotation

Codecov / codecov/patch

run_release.py#L236 

Added line #L236 was not covered by tests
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']}")

Check warning on line 239 in run_release.py

View check run for this annotation

Codecov / codecov/patch

run_release.py#L239 

Added line #L239 was not covered by tests
print()

def checkpoint(self) -> None:
Expand Down Expand Up @@ -313,17 +321,23 @@
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(

Check warning on line 324 in run_release.py

View check run for this annotation

Codecov / codecov/patch

run_release.py#L324 

Added line #L324 was not covered by tests
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(

Check warning on line 328 in run_release.py

View check run for this annotation

Codecov / codecov/patch

run_release.py#L328 

Added line #L328 was not covered by tests
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(

Check warning on line 338 in run_release.py

View check run for this annotation

Codecov / codecov/patch

run_release.py#L338 

Added line #L338 was not covered by tests
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_magic_number(db: ReleaseShelf) -> None:
release_tag = db["release"]
if release_tag.major == 3 and release_tag.minor <= 13:
Copy link
Member

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
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
Member

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
Member Author

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
Member

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):

return

Check warning on line 416 in run_release.py

View check run for this annotation

Codecov / codecov/patch

run_release.py#L416 

Added line #L416 was not covered by tests

if release_tag.is_final or release_tag.is_release_candidate:

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

subprocess.check_call(
[
"python3",
sys.executable,
"-m",
"sigstore",
"sign",
Expand Down Expand Up @@ -692,7 +709,7 @@
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"])

Check warning on line 712 in run_release.py

View check run for this annotation

Codecov / codecov/patch

run_release.py#L712 

Added line #L712 was not covered by tests
transport = client.get_transport()
assert transport is not None, f"SSH transport to {server} is None"

Expand Down Expand Up @@ -737,7 +754,9 @@
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(

Check warning on line 757 in run_release.py

View check run for this annotation

Codecov / codecov/patch

run_release.py#L757 

Added line #L757 was not covered by tests
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 @@
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(

Check warning on line 810 in run_release.py

View check run for this annotation

Codecov / codecov/patch

run_release.py#L810 

Added line #L810 was not covered by tests
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 @@
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(

Check warning on line 929 in run_release.py

View check run for this annotation

Codecov / codecov/patch

run_release.py#L929 

Added line #L929 was not covered by tests
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 @@
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"]:

Check warning on line 950 in run_release.py

View check run for this annotation

Codecov / codecov/patch

run_release.py#L950 

Added line #L950 was not covered by tests
# For security releases, only check Linux files
are_all_files_there = are_linux_files_there

Check warning on line 952 in run_release.py

View check run for this annotation

Codecov / codecov/patch

run_release.py#L952 

Added line #L952 was not covered by tests
else:
# For regular releases, check all platforms
are_all_files_there = (

Check warning on line 955 in run_release.py

View check run for this annotation

Codecov / codecov/patch

run_release.py#L955 

Added line #L955 was not covered by tests
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(

Check warning on line 966 in run_release.py

View check run for this annotation

Codecov / codecov/patch

run_release.py#L965-L966 

Added lines #L965 - L966 were not covered by tests
f"\rWaiting for files: Linux {linux_tick} (security release mode - only checking Linux) ",
flush=True,
end="",
)
else:
print(

Check warning on line 972 in run_release.py

View check run for this annotation

Codecov / codecov/patch

run_release.py#L972 

Added line #L972 was not covered by tests
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 @@
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(

Check warning on line 985 in run_release.py

View check run for this annotation

Codecov / codecov/patch

run_release.py#L985 

Added line #L985 was not covered by tests
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 @@
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
Member

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
Member

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 @@
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
Loading