Look at the apbs in the catalog for a matching name when creating a secret #438
Conversation
|
This change allowed me to create broker secrets without needing to edit the configmap when I used "apb push" to get an APB into the catalog. |
|
visual ACK |
scripts/create_broker_secret.py
Outdated
|
|
||
|
|
||
| def fqname(apb): | ||
| parts = apb.split('/') |
There was a problem hiding this comment.
Would doing parts = apb.split('/')[-1] work here?
It looks like this breaks the image apart like this: rthallisey/mediawiki123:latest -> mediawiki123:latest
There was a problem hiding this comment.
no, given either rthallisey/mediawiki123:latest or docker.io/rthallisey/mediawiki123:latest it should produce rthallisey-mediawiki123-latest
There was a problem hiding this comment.
You could do this:
if apb.count('/') >= 2:
parts = apb.split("/", 1)[-1]
parts = parts.replace("/", "-").replace(":", "-")
scripts/create_broker_secret.py
Outdated
| if len(parts) == 3: | ||
| # Chop the service, we don't know what it will be translated to in the broker | ||
| parts = parts[1:] | ||
| if ':' in parts[-1]: |
There was a problem hiding this comment.
Would this if statement ever be skipped? If it can, shouldn't latest be added to the end of parts?
scripts/create_broker_secret.py
Outdated
|
|
||
|
|
||
| def fqname(apb): | ||
| parts = apb.split('/') |
There was a problem hiding this comment.
You could do this:
if apb.count('/') >= 2:
parts = apb.split("/", 1)[-1]
parts = parts.replace("/", "-").replace(":", "-")
scripts/create_broker_secret.py
Outdated
| import sys | ||
| import yaml | ||
| import subprocess | ||
|
|
||
| try: | ||
| import yaml |
There was a problem hiding this comment.
You have import yaml, so I don't believe you would ever reach print("No yaml parsing modules installed, try: pip install pyyaml") as it stands now.
There was a problem hiding this comment.
It isn't immediately obvious why we are try/catching this. It would be helpful, to me at least, if you could explain why you are doing this.
There was a problem hiding this comment.
I'm just try-catching it because this isn't a python project and we can't really specifiy python dependencies, so I wanted a nice error message if they don't have the right dependencies installed. I messed up with the above import yaml though, oops.
scripts/create_broker_secret.py
Outdated
|
|
||
|
|
||
|
|
||
| def fqname(apb): |
There was a problem hiding this comment.
It looks like I need to make updates to this in my PR #433 since the FQName will be changing with those changes.
|
@rthallisey @djzager I think I addressed your comments. |
openshift-ci-robot
added
the
size/M
|
This has been updated to deal with auth now |
openshift-ci-robot
added
size/L
fabianvf
force-pushed
the
create-secret-update
branch
from
407bd7b
to
4e244f4
Compare
openshift-ci-robot
added
size/M
openshift-ci-robot
added
size/L
|
I removed the needs-bugzilla from this because this isn't part of the shipping broker, it's a tool used to aid testing. |
Describe what this PR does and why we need it:
Running the
./scripts/create_broker_secret.pyscript will now check the catalog for a FQName that looks like the image passed in. This should make it work with non-dockerhub registries as well as locally pushed images.Changes proposed in this pull request
/v2/catalogdepends on ansibleplaybookbundle/ansible-playbook-bundle#143