PR submitted using forked repos are failing

[arpita0911patel]

Current behavior

The current behavior of the CI pipeline is that it fails when someone uses a forked repository to submit a pull request due to DockerHub secrets not being available.

Error message at:
https://github.com/CIROH-UA/NGIAB-CloudInfra/actions/runs/7558470899
Run echo "" | docker login --username awiciroh --password-stdin
echo "" | docker login --username awiciroh --password-stdin
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
env:
TAG_NAME: merge
Error: Cannot perform an interactive login from a non TTY device

Expected behavior

CI pipeline should run successfully for forked repositories as well as pull requests submitted using branches.

Steps to replicate behavior (include URLs)

Submit a PR using a forked repo.

Screenshots

[arpita0911patel]

Lets do it this way : - on a pull request, the pipeline will build and run tests using the container on the runner in auto mode and sample test data. On merge to the main branch, the pipeline will build and push the Docker image to DockerHub and build a release.

[benlee0423]

I am trying to run the image in auto mode, and I have the following error message.
I am using AWI_03W_113060_002 and most recent image before issue #87.
Command :

docker run --rm -it -v /home/ubuntu/workspace/AWI_03W_113060_002:/ngen/data awiciroh/ciroh-ngen-image:Add-gpkg-support2 /ngen/data auto

Output:

Working directory is:
/ngen/data


Found these Catchment files:
./config/catchments.geojson


Found these Nexus files:
./config/nexus.geojson


Found these Realization files:
./config/awi_simplified_realization.json
./config/._awi_simplified_realization.json


AUTO MODE ENGAGED
Running NextGen model framework in parallel mode
Partitioning 539 catchments into 2 partitions.
Validating catchments...

Number of catchments is: 539
Catchment validation completed
Found 16 remotes in partition 0
Found 19 remotes in partition 1
Found 35 total remotes (average of approximately 17 remotes per partition)
NGen Framework 0.1.0
NGen Framework 0.1.0
Missing required argument for partition file path.
Missing required argument for partition file path.

I think some configuration issue with input file.
@ZacharyWills
Do I need to modify any section of the file in AWI_03W_113060_002/config directory?

[ZacharyWills]

It's not an issue with the configuration, it's just not finding the partition file that it made for some reason.

NGIAB-CloudInfra/docker/HelloNGEN.sh

Line 59 in da87260

mpirun -n $procs /dmod/bin/ngen-parallel $selected_catchment all $selected_nexus all $selected_realization $(pwd)/partitions_$procs.json

On the end here in auto mode it's expecting that file to be generated in the same directory that it's going to be run from.

try: docker run --rm -it -v /home/ubuntu/workspace/AWI_03W_113060_002:/ngen/ngen/data awiciroh/ciroh-ngen-image:Add-gpkg-support2 /ngen/ngen/data auto

All I added was an update to the directory we use for the data stream (using /ngen/ngen/data instead of /ngen/data.

Not sure if this will fix it but when I tried it here locally again it worked.

[benlee0423]

Getting the same error message after changing /ngen/data to /ngen/ngen/data.

[ZacharyWills]

Getting a new error when I run the old image as well. I'll have a longer look tomorrow.

[benlee0423]

Running the same image in non-auto mode, and it runs successfully.

Command:

docker run --rm -it -v /home/ubuntu/workspace/AWI_03W_113060_002:/ngen/ngen/data awiciroh/ciroh-ngen-image:Add-gpkg-support2 /ngen/ngen/data

Output:

.....
[539 rows x 8640 columns]
2024-01-19 04:52:08,719   DEBUG [output.py:398 - nwm_output_generator()]: output complete in 4.573228120803833 seconds.
2024-01-19 04:52:08,719   DEBUG [__main__.py:277 -             main_v04()]: process complete in 11.395111560821533 seconds.
Finished routing
NGen top-level timings:
	NGen::init: 15.119
	NGen::simulation: 13.3655
	NGen::routing: 11.4121
************ TIMING SUMMARY ************
----------------------------------------
Network graph construction: 0.99 secs, 8.71 %
Forcing array construction: 3.31 secs, 29.08 %
Routing computations: 2.51 secs, 22.04 %
Output writing: 4.57 secs, 40.14 %
----------------------------------------
Total execution time: 11.379999999999999 secs

real	0m41.089s
user	1m39.064s
sys	0m11.897s

[ZacharyWills]

Rebuilt the image on our AWS instance from scratch (the existing docker files) and updated the HelloNGEN.sh file to look for gpkg and it worked just fine.

[ZacharyWills]

Also how was awiciroh/ciroh-ngen-image:Add-gpkg-support2 built? There's no files for it or commit IDs or anything, I have no idea where in the timeline of the repo this image was created (it's significantly older than the latest, which is working for me). If the latest works with auto mode, I'm not sure why we need to test this old, one-off image after the fact.

[benlee0423]

I would not recommend build locally.
If you get latest code and build in Runner, all four images ( ngen-dep, troute, ngen, ciroh-ngen-image) have the same tag.
But, if you do build locally or AWS instance, please make your build commands are using the same image tag name for all three images ( ngen-dep, troute, ngen) and ciroh-ngen-image.

[benlee0423]

If the latest works with auto mode, I'm not sure why we need to test this old, one-off image after the fact.

Current latest image does not work due to issue #87 and #80, I was searching the most latest image tag which have your auto-mode change, but not having PR #80.

[benlee0423]

There are some security concerns proceeding to fixing this issue. Please look at the below articles.
In short,
there exists a potentially dangerous misuse of the pull_request_target workflow trigger that may lead to malicious PR authors (i.e. attackers) being able to obtain repository write permissions or stealing repository secrets.
Also, start and stop Runner would not work from public fork because it also requires secrets.
This fix requires rewriting entire CI workflows and addressing any security issues and tests, which requires a lot of time and effort.

https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/enabling-features-for-your-repository/managing-github-actions-settings-for-a-repository#enabling-workflows-for-forks-of-private-repositories

And,
https://securitylab.github.com/research/github-actions-preventing-pwn-requests/

[benlee0423]

#103

[benlee0423]

PR from forked branch verified in Actions.
https://github.com/CIROH-UA/NGIAB-CloudInfra/actions/runs/7922998852
Resulting image from this build also worked.