Skip to content

[kots]: put the HTTP Proxy settings behind a KOTS config bool #13959

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

Merged
merged 1 commit into from
Oct 19, 2022

Conversation

mrsimonemms
Copy link
Contributor

@mrsimonemms mrsimonemms commented Oct 18, 2022

Description

Add a checkbox to enable the HTTP proxy settings. The values are still taken from the KOTS CLI input variables, but will not be applied until the option is configured

image

Related Issue(s)

How to test

Install via KOTS. Run the command kubectl get deployments.apps -n gitpod blobserve -o yaml | grep -i HTTP_PROXY -a2 - unless you have the checkbox enabled, this should return nothing.

Release Notes

[kots]: put the HTTP Proxy settings behind a KOTS config bool

Documentation

Werft options:

  • /werft with-local-preview
    If enabled this will build install/preview
  • /werft with-preview
  • /werft with-integration-tests=all
    Valid options are all, workspace, webapp, ide

@mrsimonemms
Copy link
Contributor Author

/hold I'd like for @mrzarquon's approval as well

@mrzarquon
Copy link
Contributor

mrzarquon commented Oct 18, 2022

(reposting from Slack) - shouldn't this be in the installer config setting itself? We're not planning to support any customer questions about this option yet, so we shouldn't be putting it in the UI. Requiring an explicit "enable: true" prevents us from doing the rest of the processing, allows us to expose the value if we want via a config patch like this below.

httpProxy:
  enable: true
  envvars:
    kind: secret
    name: http-proxy-settings

What happens for customers already on 2022.09 who have a http-proxy-settings created by previous release - does the current kots process remove it when it upgrades or will it be ignored and left for us to trip over because KOTS only manages things when it is told to (and it's only told to if the KOTS ui checkbox is used)?

@mrsimonemms
Copy link
Contributor Author

mrsimonemms commented Oct 19, 2022

@mrzarquon I don't think it should be in the config block. The HTTPProxy block in the config has an implicit enabled by having a value in there (which is why it's a pointer). It's also using the ObjectRef block which is explicitly a secret block and is used in many different places.

We could change how it works (it's not a supported property at the moment, so there shouldn't be anyone using it out there in the real world), but it'll take more time than we have at the moment to get it out as a hotfix.

If someone has the http-proxy-settings secret created by a previous release, this will be deleted with a new release (I tested that as part of this work yesterday) so it will remove that secret.

@mrzarquon
Copy link
Contributor

If that's the case, if can we hide it for now, either with hidden: true without the when value, or put it in an experimental group that is hidden entirely, then I'd be ok with this going ahead.

- name: experimental
  title: Experimental Options
  description: You shouldn't be seeing these in the UI
  when: false
    items:
      - name: enable_proxy_settings
        title: Enable proxy settings
        type: bool
        default: "0"
        help_text: Enable HTTP_PROXY support for the Gitpod application. This is currently an experimental feature.

@mrsimonemms
Copy link
Contributor Author

Yeah, can definitely go with that m'colleague

@mrsimonemms mrsimonemms force-pushed the sje/http-proxy-conditional branch from 96aa632 to 192667d Compare October 19, 2022 10:52
@mrsimonemms
Copy link
Contributor Author

@mrzarquon have put this in the experimental config block as suggested and then disable this config block from being displayed.

As before, this deletes the http-proxy-settings secret from the cluster

@mrsimonemms
Copy link
Contributor Author

/unhold

Copy link
Contributor

@ArthurSens ArthurSens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes seem to be aligned with what was described in the PR description 🙂

I just made one question, but it can be answered after merge!

Comment on lines +11 to +12
annotations:
kots.io/when: '{{repl ConfigOptionEquals "enable_proxy_settings" "1" }}'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you point me to docs that explain what this annotation does?

Copy link
Contributor Author

@mrsimonemms mrsimonemms Oct 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This includes a resource in the KOTS release, in this case if the enabled_proxy_settings config value equals 1 - it'll either resolve true or false and the annotation includes it if it's true

Docs

@roboquat roboquat merged commit 0cf04fb into main Oct 19, 2022
@roboquat roboquat deleted the sje/http-proxy-conditional branch October 19, 2022 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants