-
Notifications
You must be signed in to change notification settings - Fork 11
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
AWS admin sign-up automation #45
Conversation
Committing email-config.py.
Just pass the name of the file into terminal as an argument. Script should do the rest.
First draft of the script is working and ready for review :) |
@shankari I realized you're not mentioned here, so probably didn't get my update! The script is ready for review 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nataliejschultz I don't see any indicating of the testing done.
I understand that you might not have automated tests for this, but I don't even see any indication of manual testing done. In the future, please indicate testing done before sending it to me for review so I know that it is worth reviewing
Other changes below...
Push addresses the clean-up changes requested. Adds a main function for code that should not be run outside of the file. Adds authentication for sts client. Simplifies get_userpool_name and user_already_exists functions Updates format_email function to add further customization of welcome email based on admindash_prefs section of config
Removing an unnecessary print
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks much better. I have some suggested text changes here.
I think that this should be ready to merge now as a standalone script - why is this still in draft mode?
@nataliejschultz if it is ready to merge, please move it out of draft.
After this is merged, you can create a separate PR for the GitHub action, which can use OIDC/IAM to log in.
That follow-on PR should also change the intake form to allow participants to specify admin users
- Added section to README emphasizing the need to install boto3 + other instructions on manually running script. - Updated welcome email template's admindash link, as well as modified sentences per Shankari's suggestions. - email-config.py: Removed comment, updated how email addresses are accessed in the config based on Abby's changes, and updated more welcome email language based on Shankari's suggestions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- One real cleanup (the URL for the admin).
Multiple future cleanups. I would not have held up the release just for these, but since I am sending the PR back anyway, please get them handled as well.
main.yml will work with our IAM role in AWS to connect Cognito and SES with GitHub Actions
This workflow should run when a new file is added to the config directory. The dependency (Get config name) should get the name of the newly added file, which is passed in during Run email-config.py.
Updating the url to the admin dashboard in the welcome email.
Adding a line about installing boto3 in a venv or conda env as opposed to implying that it must be installed on the user's local machine.
I would like you to support both. It sounds like the main difference is in authenticating to AWS. So you would have |
* email-config.py: 1. Adds support for -l vs -g args Modified client setup + get_verified_art function to be compatible with the specified arg. 2. Fixes issue with link to admin dashboard. It works now :) 3. Removed a section of the update_user_pool function per Jianli's request, since it was unnecessary. 4. Modified to allow full path to config file as input * Updated README to reflect the local run `-l` arg, as well as the path to the config file. * Removed the two workflow files, since they will be part of a different PR.
Updates to the script due to the work going on to automate the script. Mainly updates working around the file path issues I've encountered in GitHub Actions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there!
using filepath_raw instead of relpath
removed print!
@nataliejschultz This looks good to me overall and I am ready to merge it after the task below. |
Path to the welcome template file needs to use maindir variable for both -l and -g runs.
I found an error with running the script from different directories while testing. Testing is now done, the script is updated, and the PR is back in ready for review! I think it's all ready. |
Updating to be up-to-date with latest push in the [other PR](#45)
@nataliejschultz wrt "Can you please update with "Testing done"" I would like you to actually list the testing done. .Testing from the same directory
more tests |
I removed myself from the account between each test to see if it would get through the entire process: running script directly:
running from one directory up:
running at config file location:
running from far away directory:
I got an email each time! |
Fixing the parsing issue for emails that we encountered during demo!
I added functionality to remove users who are not in the config file, but are already in the user pool. Previously, I created the
this function takes For the new use case, I modified it a bit to create
This function is called early on in the program, and gives us the Next, each dictionary in
If the
I tested this by adding myself to the pool:
Then trying to re-add myself AND another new email,
And finally, removing my first email from the config to see what happens:
I checked the user pool, and my email1@nrel.gov was indeed removed from the pool! I decided to try and follow the welcome email to log in with my removed email, just to see what would happen. I got the following message: |
@shankari I haven't pushed these changes yet since this PR is still open; I am happy to put it in a separate PR once this one is merged, or just push it to this one. Wanted to check with you beforehand. |
* Test push :) * Init commit for main.yml + email-config.py main.yml will authenticate AWS with our IAM role, use a dependency to get the name of the latest file pushed to configs directory, and run email-config.py. Pushing email-config.py as it was in the last commit to my nataliejschultz:AWS-email-config PR. Reverting README.md to before test push. * configs push test Testing to see if pushing to the configs directory runs the main.yml workflow! * Updating main.yml main.yml didn't run on my last push (though I thought it would). Updating when the workflow runs and trying again. * Another test push Trying to get workflow to run. If this doesn't work, it might be because my PR is a draft? I've seen conflicting info about how to get a workflow to run on a non-main branch in PR phase. * Region error trying to fix `Error: Region is not valid: <"us-west-2"> ` * Push test Changing file to trigger workflow. * Fixing typo Typo was causing secrets access issue. * Push test * Changing ARN syntax + push test Hoping that previous run is just a syntax error that I'm correcting now. * Changing option in run + push test `m` is not an option for the command, so I added a different one. * Creating clients + push test I'm not sure how to set up the client when authenticating through IAM. Trying this out. * Modifying cognito client + push test Adding region to cognito client + moving where environment variable is accessed. * Combining jobs to pass credentials I don't think the jobs can be separated if I want to pass the credentials from my AWS auth step to the run step. Going to try combining them and see what happens! * Fixing dependency I don't think this dependency worked because of where `id` was in the workflow. Trying again! * Testing TJ dependency Testing out another dependency to see if it's better than jitterbit :) * Push test Testing out TJ dependency with this push! * Config file export Modified the `changed_files` job to include a for loop that exports an environment variable with the name of the changed file. The variable is then passed in while running the python script. Hoping this works! * Push test * Typo Forgot to change value of file from changed_file. Oops. * Push test * Adding prints Adding some prints for troubleshooting to see if the name of the config file is being passed properly or not. * Push test * Trying out another way to access config file Accessing environment variable (hopefully) the right way. * Trying to get jobs to run sequentially * Another way to pass config file name Trying another way to pass the config file name between the two jobs. * Prints prints to figure out why the filepath isn't correct * Another print Adding another print. * Config relative file path change The relative path is slightly different in github actions. Adding an if else line to change depending on -l or -g arg * Using full path to file Relative paths seem to have an issue in GHA. trying full path to file. * Push test after updated settings Last error was due to AWS permissions not working. Jianli updated the settings, so we're going to check and see if it works now. * Push test Trying again per jianli * Updating config path for -g + removing prints Last run worked! I removed myself from the user pool, so I'm going to try again with this updated method of getting the config path for -g runs + trying to actually add myself to the user pool. * Adding welcome template Forgot that I needed to add the welcome template file! Adding :) * File not found? Got `FileNotFoundError: [Errno 2] No such file or directory: 'welcome-template.txt'` on last run. Pushing a change to make sure the file is there and going to try again. * Push test * Adding pwd Added os.getcwd() to see why it's not opening the welcome template file. * welcome template access adding a workaround for the filepath issue * Filepaths again Trying another fix. * Previous issue fixed. Now sts client Adding an empty defintion for sts_client with -g since it has to be passed into the get_verified_arn function. * Push test Testing now that the AWS settings have been updated ! * Updating script per changes +emoving changes to readme Updating email-config so it's up to date with the other email-config.py in the other PR. Removing the PR's changes to the Readme (not sure how they got on there) * Readme Reverting readme? * Updating email-config.py Updating to be up-to-date with latest push in the [other PR](#45) * Renaming workflow Giving main.yml a more meaningful name (AWS-auth.yml) * Push test testing workflow under new name before removing email :) * Removing email Removing email address from wyoming file. Workflow will probably raise an error on this run. * Trying to fix readme changes * Trying to remove all changes to readme I tried git checkout, but that didn't give me a pushable option to remove my previous commit changes. Let's see how this does :) * Removing duplicate files Since the final test worked fine, we can remove them from this PR. They should be merged on the other PR first, and then this PR can be closed. * rename for demo * re-adding files for demo * adding wyoming config file * Email parsing fix After demoing with Abby, we found out that the emails weren't being parsed correctly due to differences in our previous configs vs the new generation method. Fixing and testing! * Delete wyoming.nrel-op.json * Add wyoming config * Restoring wyoming original config Restored the old wyoming config + removing files that were temporarily re-added for the demo * What happens when modifying two configs in one push? Trying to see what happens when we update two config files at once. Will the value of the github output be formatted as a string? Will it be a comma separated list? * echo multiple filename change output Checking output when we change multiple configs at once * push test * Push test Trying to activate the GH actions workflow * Changing bash filename handling Modifying the GHActions script to (potentially) handle multiple changed config files. * Array appending debugging Array did not work for bash. Checking out it's being output. * Array formatting Modifying array formatting so it's hopefully what I want! Also seeing if I can read the actual output, though it might produce a context error. * Moving things around moving some echoes. * Array syntax Seeing if the array will echo properly with this syntax change. * Testing passing two changed files Modified the GH Actions to be able to pass in and loop over two config files. We'll see if it works by temporarily adding in email-config.py with a relevant print and seeing what happens! * Syntax? I'm not sure what went wrong the first time with bash. Adding some echoes to figure it out. * More subtle bash syntax As it would turn out, bash scripting is finicky when dealing with arrays. My array is not being sent to the GitHub output properly for some reason. Trying it out by replacing * with @. If that doesn't work, I'm going to try removing the entire [@] section and see what happens. * More syntax, whoops! I changed CONFIG_FILE to CONFIG_FILES at one point and didn't change it in the outputs section. Now let's see if it works! * Bug catching Caught a bug in my email-config.py script where it wasn't returning is_userpool_exist properly. Let's see if this fixes things. * Commenting out Want to merge recent changes to my branch, but don't want to re-run all these configs! * renaming email config renaming to merge from main. * email-config.py bug fix Fixing a bug I found a few pushes ago. * Two changed files, removal test Testing removal of my email from the wyoming pool, in addition to adding myself to nrel-commute pool in the same action. * See last commit. Running job Job wasn't picked up on last push. Trying again. * action run only on push to main changing branch that workflow runs on push to (main) * Reverting wyoming config Reverting Wyoming config file * WY format Continuing reverting WY file * syntax blank space removal * Update wyoming.nrel-op.json Adding blank line after file * Reverting commute config Reverting to original commute config
PR for e-mission/e-mission-docs#1008
email-config.py currently:
will: