redhat-cop / openshift-applier

Used to apply OpenShift objects to an OpenShift Cluster
Apache License 2.0
102 stars 61 forks source link

check_file_location in filter_plugin might be doing too much #100

Open etsauer opened 5 years ago

etsauer commented 5 years ago

We've addressed immediate issues like #91 with PR #97, so this isn't urgent. However, I still believe that the filter_plugin methods should not attempt to verify that a URL returns a healthy response. Specifically, there are two cases we are likely to hit in the future where this will create false negatives:

  1. Unexpected return codes
    • In cases where we are either operating in environments behind a proxy, or pointing at a location behind proxies with unexpected configs, its possible we could get a non 200 response code that will work perfectly fine when passe to oc
  2. Untrusted CAs
    • It's reasonable to expect a consumer of applier may be hosting remote yml files on servers where Python may not trust the CA, but oc does. Think gogs running on openshift with the openshift signed, or customer signed certificate

For this reason, I think that the functionality within the check_file_location should only check for the characteristics of a URL (scheme, hostname, path) instead of actually trying to follow all URLs passed by the user.

oybed commented 5 years ago

@etsauer so I think this may fit in with a bigger refactor/re-design of the openshift-applier. Maybe we can take a step back to consider some higher level architecture changes, including:

I think if we work the above, the items outlined in this issue may become resolved as well.