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 provider mail param #12

Merged
merged 3 commits into from Feb 26, 2019
Merged

Add provider mail param #12

merged 3 commits into from Feb 26, 2019

Conversation

miguelsorianod
Copy link
Contributor

Make the provider admin mail configurable when a
deploy is performed via a parameter
named ADMIN_EMAIL, and the USER_EMAIL system envvar
is set from that value.

@miguelsorianod
Copy link
Contributor Author

miguelsorianod commented Jan 2, 2019

The commits should make this possible but I think it would not work in all cases.

When you define an environment variable in a container in a template, then it is defined, and thus it always has a value. That's the case even if no value is provided to the environment variable at template level, in which case the environment variable is set with the empty string value.

This means that when creating the USER_EMAIL environment variable at template level, which is the one used by the porta code, and assigning it the value of a ConfigMap entry named 'ADMIN_EMAIL' (the value of which is obtained from an OpenShift parameter with the same name), when the parameter is not provided then the environment variable is set to the empty string.

The porta code that handles the definition of the USER_EMAIL value is in https://github.com/3scale/porta/blob/master/db/seeds.rb#L129.
There it can be seen that a default value is assigned in case the variable is not defined, but in the case of a template deploy this can never happen at template level because when the "environment variable" has no value, it is still defined with the empty string value. This implies that a default value is never set in this case, which would be incorrect.

I verified this by applying the changes, deploying AMP, accessing the 'system-master' container and looking for the value of the 'USER_EMAIL' environment variable, and I can see it is defined but with the empty value when no email is provided at deployment time:

oc new-app amp-eval.yml  --param WILDCARD_DOMAIN=msoriano.127.0.0.1.nip.io
sh-4.2$ printenv | grep -i user_email
USER_EMAIL=
sh-4.2$

The options I see are:

  • We set a default value in the template OpenShift parameter. This implies that the default value will really never be set at code level when deploying AMP because the variable will ALWAYS exist when an AMP deployment is performed (even if the parameter is not set, in that case the envvar will still exist as the empty string)
  • We check at porta code level whether the email variable is empty or not. I guess this is the same that what is done in the SMTP notifications fields at porta code level too?

cc @guicassolato @hallelujah

@guicassolato
Copy link
Contributor

We can make porta's code to check whether a value is present (as opposed to provided as an empty string or not provided at all).

Make the provider admin mail configurable when a
deploy is performed via a parameter
named ADMIN_EMAIL, and the USER_EMAIL system envvar
is set from that value.
@miguelsorianod miguelsorianod changed the title [WIP] Add provider mail param Add provider mail param Feb 25, 2019
@codeclimate
Copy link

codeclimate bot commented Feb 26, 2019

Code Climate has analyzed commit eca080a and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Style 1

View more on Code Climate.

@miguelsorianod miguelsorianod merged commit badb20a into master Feb 26, 2019
@miguelsorianod miguelsorianod deleted the add-provider-mail-param branch February 26, 2019 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants