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

fix: move conf/cert to t/certs and disable ssl by default #2112

Merged
merged 32 commits into from
Nov 20, 2020

Conversation

Yiyiyimu
Copy link
Member

What this PR does / why we need it:

fix #2110

Pre-submission checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible?

@moonming moonming added this to the 2.1 milestone Sep 16, 2020
@membphis
Copy link
Member

@Yiyiyimu please fix the issue of CI

@Yiyiyimu
Copy link
Member Author

Hi @membphis the CI error is due to there're some problems unsolved. Discussion is on the original issue #2110.

Signed-off-by: yiyiyimu <wosoyoung@gmail.com>
Signed-off-by: yiyiyimu <wosoyoung@gmail.com>
Signed-off-by: yiyiyimu <wosoyoung@gmail.com>
Signed-off-by: yiyiyimu <wosoyoung@gmail.com>
Signed-off-by: yiyiyimu <wosoyoung@gmail.com>
Signed-off-by: yiyiyimu <wosoyoung@gmail.com>
…le_ssl'

Signed-off-by: yiyiyimu <wosoyoung@gmail.com>
Signed-off-by: yiyiyimu <wosoyoung@gmail.com>
Signed-off-by: yiyiyimu <wosoyoung@gmail.com>
Signed-off-by: yiyiyimu <wosoyoung@gmail.com>
Signed-off-by: yiyiyimu <wosoyoung@gmail.com>
Signed-off-by: yiyiyimu <wosoyoung@gmail.com>
Signed-off-by: yiyiyimu <wosoyoung@gmail.com>
Signed-off-by: yiyiyimu <wosoyoung@gmail.com>
@membphis membphis changed the title fix: move conf/cert to t/certs chore: move conf/cert to t/certs Nov 18, 2020
Signed-off-by: yiyiyimu <wosoyoung@gmail.com>
Signed-off-by: yiyiyimu <wosoyoung@gmail.com>
Signed-off-by: yiyiyimu <wosoyoung@gmail.com>
Signed-off-by: yiyiyimu <wosoyoung@gmail.com>
Signed-off-by: yiyiyimu <wosoyoung@gmail.com>
Signed-off-by: yiyiyimu <wosoyoung@gmail.com>
Signed-off-by: yiyiyimu <wosoyoung@gmail.com>
@Yiyiyimu
Copy link
Member Author

Yiyiyimu commented Nov 19, 2020

Kind of stuck here and need some help. Thx~

@@ -61,7 +61,7 @@ apisix:
- 127.0.0.0/24 # If we don't set any IP list, then any IP access is allowed by default.
# - "::/64"
# port_admin: 9180 # use a separate port
https_admin: true # enable HTTPS when use a separate port for Admin API.
# https_admin: true # enable HTTPS when use a separate port for Admin API.
Copy link
Member

Choose a reason for hiding this comment

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

What's the default value?

Copy link
Member Author

@Yiyiyimu Yiyiyimu Nov 19, 2020

Choose a reason for hiding this comment

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

nil I think

@nic-chen
Copy link
Member

@Yiyiyimu

It is another issue now.

I think it is caused by ssl not being enabled by default:
https://github.com/apache/apisix/pull/2112/files#diff-8d872babc717e9d733641b56bfc530ef98751fbe4e68f08d79b2b83109c22fffR105

You can modify the test case accordingly and move on.

Signed-off-by: yiyiyimu <wosoyoung@gmail.com>
@Yiyiyimu
Copy link
Member Author

Thank you for the fix and problem pointing out @nic-chen! All test passed and ready to be reviewed~

Signed-off-by: yiyiyimu <wosoyoung@gmail.com>
enable_http2: true
listen_port: 9443
# ssl_trusted_certificate: /path/to/ca-cert # Specifies a file path with trusted CA certificates in the PEM format
# used to verify the certificate when APISIX needs to do SSL/TLS handshaking
# with external services (e.g. etcd)
ssl_cert: ""
Copy link
Member

Choose a reason for hiding this comment

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

Here is the path or content of the certificate? we need to add comments.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you! fixed

@Yiyiyimu Yiyiyimu changed the title chore: move conf/cert to t/certs fix: move conf/cert to t/certs and disable ssl by default Nov 20, 2020
@moonming
Copy link
Member

@nic-chen please take a look

@moonming moonming merged commit 36162e3 into apache:master Nov 20, 2020
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.

request help: move conf/cert to t/cert
6 participants