cnti-testcatalog / testsuite

📞📱☎️📡🌐 Cloud Native Telecom Initiative (CNTI) Test Catalog is a tool to check for and provide feedback on the use of K8s + cloud native best practices in networking applications and platforms
https://wiki.lfnetworking.org/display/LN/Test+Catalog
Apache License 2.0
169 stars 70 forks source link

Allow GH actions on PRs from forked repos #2080

Closed martin-mat closed 5 days ago

martin-mat commented 1 week ago

Make "docker login" in spec execution optional. Note that it has anyway limited (if any) effect as docker registry mirror cache is used and docker login does not really have expected affect. The registry mirror cache itself needs to be configured with respective credentials.

Issues:

Refs: #2066

How has this been tested:

Types of changes:

Checklist:

Documentation

Code Review

Issue

horecoli commented 1 week ago

What about registry_spec.cr where docker login is mandatory? There is code like:

if ENV["DOCKERHUB_USERNAME"]? && ENV["DOCKERHUB_PASSWORD"]? result = Dockerd.exec("docker login -u $DOCKERHUB_USERNAME -p $DOCKERHUB_PASSWORD", force_output: true) Log.info { "Docker Login output: #{result[:output]}" } else puts "DOCKERHUB_USERNAME & DOCKERHUB_PASSWORD Must be set.".colorize(:red) exit 1 end

martin-mat commented 1 week ago

What about registry_spec.cr where docker login is mandatory? There is code like:

if ENV["DOCKERHUB_USERNAME"]? && ENV["DOCKERHUB_PASSWORD"]? result = Dockerd.exec("docker login -u $DOCKERHUB_USERNAME -p $DOCKERHUB_PASSWORD", force_output: true) Log.info { "Docker Login output: #{result[:output]}" } else puts "DOCKERHUB_USERNAME & DOCKERHUB_PASSWORD Must be set.".colorize(:red) exit 1 end

Good point. Currently it passes because DOCKERHUB_USERNAME are set (but empty string) so the condition is true. Docker login fails but is ignored. At the end the test passes, despite not being logged in to dockerhub.