+
Skip to content
This repository was archived by the owner on Sep 28, 2021. It is now read-only.

Conversation

axelssonHakan
Copy link
Member

Rename certificates to tlsClientConfig since it will configure both:

  • certificates
  • InsecureSkipVerify

The tlsClientConfig will also be created by default now (possible problem?)

The path for connecting to engine is set to "/app/engineData/ttl/" for allowing pass-thru for Qlik Sense Proxy.

An User-Agent header corectl/<version> (<os>) is added to be able to track connections in ingress etc

log.Verboseln("SessionId " + headers.Get("X-Qlik-Session"))

var dialer = enigma.Dialer{}
dialer.TLSClientConfig = tlsClientConfig
Copy link
Contributor

Choose a reason for hiding this comment

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

Good simplification.

Copy link
Contributor

@glooms glooms left a comment

Choose a reason for hiding this comment

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

Generally looks good, nice addition. Just wondering why we changed the default web-socket url.

Copy link
Member

@wennmo wennmo left a comment

Choose a reason for hiding this comment

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

Looks good, some minor comments

cmd/flags.go Outdated
globalFlags.Bool("bash", false, "Bash flag used to adapt output to bash completion format")
globalFlags.MarkHidden("bash")
globalFlags.String("context", "", "Name of the context used when connecting to Qlik Associative Engine")
globalFlags.BoolP("allow-insecure", "i", false, "Enabling allow-insecure will make it possible to connect using self signed certs")
Copy link
Member

Choose a reason for hiding this comment

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

In my opinion we should skip the shorthand flag, since it is not recommended to ignore certificate validation (even if we are using self signed certificates. Also change certs -> certificates

docs/spec.json Outdated
"clispec": "0.1.0",
"x-qlik-stability": "stable",
"flags": {
"allow-insecure": {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe also do a patch bump on the api spec version?

@axelssonHakan axelssonHakan changed the title DRAFT: Allow insecure connections (self signed cert) Allow insecure connections (self signed cert) Dec 4, 2019
@axelssonHakan axelssonHakan requested a review from wennmo December 4, 2019 09:52
Copy link
Member

@wennmo wennmo left a comment

Choose a reason for hiding this comment

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

LGTM

@axelssonHakan axelssonHakan merged commit 632b126 into master Dec 9, 2019
@axelssonHakan axelssonHakan deleted the enableInsecureConnection branch December 9, 2019 09:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载