Skip to content
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

Add defaulting NAMESPACE and OPENSHIFT_USERNAME parameters #16021

Conversation

@jim-minter jim-minter self-assigned this Aug 28, 2017
@openshift-merge-robot openshift-merge-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 28, 2017
@openshift-merge-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jim-minter
We suggest the following additional approver: mfojtik

Assign the PR to them by writing /assign @mfojtik in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@jim-minter
Copy link
Contributor Author

jim-minter commented Aug 28, 2017

@bparees @openshift/api-review ptal

Requirement: capability to substitute current namespace and username into templates at instantiation time without having to specify values manually.

It's proposed only to implement this on the TemplateInstance API, not the back-end of oc process, as the end namespace and username are not known at oc process time (although we could theoretically late bind on the backend of oc create instead, erk).

There are at least a couple of potential approaches here:

  1. substitute into NAMESPACE and OPENSHIFT_USERNAME parameters (names to be reviewed) when these exist as parameters and have no value specified in the template or overridden by the user
  2. substitute into names which can't exist as parameters, e.g. "${OPENSHIFT:NAMESPACE}", "${OPENSHIFT:USERNAME}" [currently parameters may not include the : character]

Some pros and cons:

  1. parameters can be manually specified in the oc process case, and can also be overridden; however via web UI end users must somehow know to leave these boxes empty in a common case instantiation
  2. no risk of stomping existing templates, but templates containing these parameters would be incompatible with oc process | oc create -f -. No overrides.

@openshift/api-review, wdyt?

@bparees
Copy link
Contributor

bparees commented Aug 28, 2017

(2) is also incompatible w/ a non-service-catalog(tsb) based web console template provision flow.

@smarterclayton
Copy link
Contributor

I think we have to be hard compatible - i.e. someone has to ask for these to be special replaced. I don't like "OPENSHIFT" existing in the name. These really sound like generators, if we do them.

What things need namespace (I.e. list use cases for namespace and username here)? We've said upstream that certain things should always be done via downwards API. Knowing what is not covered by downwards API is good.

@jim-minter
Copy link
Contributor Author

@smarterclayton
in #8971 @sosiouxme wanted to refer to a service account in the template namespace in a ClusterRoleBinding.
in #13934 it was wanted for a DeploymentConfig imagechange trigger to refer to a imagestream in the "current" namespace. Perhaps the template was creating a DC in a second namespace and wanted to refer back?
in https://bugzilla.redhat.com/show_bug.cgi?id=1331268 two customers wanted to set the Route name to include the "current" namespace, but not according to the way we autogenerate it.
in https://bugzilla.redhat.com/show_bug.cgi?id=1445146 a customer wanted the current user to be able to customise configuration deployed by the template, e.g. have the deployed app configured to email updates to the deploying user.

@smarterclayton
Copy link
Contributor

smarterclayton commented Aug 29, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-api-review size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants