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

chore: remove sysadmin part2 #1717

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

katebygrace
Copy link
Contributor

JIRA:CLOUDSEC-12

Copy link
Contributor

@matthugs matthugs left a comment

Choose a reason for hiding this comment

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

I don't think this will work. The contents of the devops/jenkins folder doesn't automatically end up in jobs' workspaces. Lines like

shell(dslFactory.readFileFromWorkspace('devops/resources/saml-ssl-expiration-check.sh'))
don't add anything to the workspace; they dump the contents of that file into the job's configuration, like this:
contents of devops/resources/saml-ssl-expiration-check.sh viewed inside the configure panel of the resulting job in the jenkins UI

I think we need to get these files cloned into the workspace from somewhere. It doesn't have to be from edx-ops/sysadmin per se, but we have to clone something (or embed the python code in the shell file or some such hacky replumbing).

I'm not sure I understand the rationale for trying to remove references to the sysadmin repo in the first place; is there a more specific ticket you can reference for which work this pertains to? Could we create a repo specifically for scripts we're migrating out of edx-ops repos we'd like to delete off Github? Or put these in https://github.com/openedx/configuration/tree/master/util like the sysadmin README suggests?

Copy link
Contributor

Choose a reason for hiding this comment

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

we probably want to add some txt files to the .gitignore in this repo if we copy these into here


env
set -x

cd $WORKSPACE/sysadmin
pip install -r requirements/base.txt
pip install -r ../eequirements/base.txt
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
pip install -r ../eequirements/base.txt
pip install -r ../requirements/base.txt

@katebygrace
Copy link
Contributor Author

re: rationale; sysadmin has credentials in it, so when we clone that onto jenkins we're back in plaintext password land; which we're moving off of, that said will give this a look over

@matthugs
Copy link
Contributor

matthugs commented Jan 3, 2024

re: sysadmin having credentials in it, great that makes sense & is what I expected you to say.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants