Skip to content
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.

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

Better package name matching for Nexus #1447

Merged
merged 2 commits into from
Apr 19, 2023

Conversation

craddm
Copy link
Contributor

@craddm craddm commented Apr 18, 2023

✅ Checklist

  • You have given your pull request a meaningful title (e.g. Enable foobar integration rather than 515 foobar).
  • You are targeting the develop branch.
  • Your branch is up-to-date with the develop branch (you probably started your branch from develop but it may have changed since then).
  • If-and-only-if your changes are not yet ready to merge, you have marked this pull request as a draft pull request and added '[WIP]' to the title.
  • If-and-only-if you have changed any Powershell code, you have run the code formatter. You can do this with ./tests/AutoFormat_Powershell.ps1 -TargetPath <path to file or directory>.

⤴️ Summary

Modifies how the configure_nexus.py script creates Nexus Content Selector search patterns, allowing CRAN packages to include capital letters and following the complete package name with _ (e.g. RPostgreSQL_) to prevent overly permissive matching and installation of non-allow-listed packages.

Note that to perform the fix I had to teardown the existing proxy VM and redeploy it. The issues cannot be fixed by modifying the local copy of the configure_nexus.py script, as the important copy is on the docker container running Nexus on the VM. It's not clear to me how to modify that without redeploying the VM entirely (but it may be clear to someone else!).

🌂 Related issues

Closes #1445
Closes #1439

🔬 Tests

Packages with capital letters can now be installed correctly on Tier 3 VMs:
Screenshot 2023-04-18 at 17 52 10

Packages that are not on the allowlist but that had names that would still match search patterns for other packages (e.g. csvread, which matched the pattern for csv) can no longer be installed and will correctly return a 403 Forbidden code:
Screenshot 2023-04-18 at 17 52 29

Copy link
Member

@JimMadge JimMadge left a comment

Choose a reason for hiding this comment

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

Excellent, two birds with one stone!

@JimMadge JimMadge merged commit 13fd751 into alan-turing-institute:develop Apr 19, 2023
@edwardchalstrey1
Copy link
Contributor

Note that to perform the fix I had to teardown the existing proxy VM and redeploy it. The issues cannot be fixed by modifying the local copy of the configure_nexus.py script, as the important copy is on the docker container running Nexus on the VM. It's not clear to me how to modify that without redeploying the VM entirely (but it may be clear to someone else!).

@JimMadge is there any reason we shouldn't just go ahead and do this for prod4? At the moment the only tier 3 SRE is Edon so it seems like the only consideration is a small amount of downtime for R package installation

@JimMadge
Copy link
Member

@edwardchalstrey1 I don't think that should be a problem.

@craddm craddm deleted the nexus-issues branch May 15, 2023 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants