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

feat:add higress automatic https #854

Merged
merged 17 commits into from
May 9, 2024
Merged

Conversation

2456868764
Copy link
Collaborator

@2456868764 2456868764 commented Mar 1, 2024

Ⅰ. Describe what this PR did

在Higress控制面集成支持域名静态配置 和 支持secret对接lets encrypt自动续签,在higress-system namesapce 新增一个 configmap ,其配置格式如下

apiVersion: v1
kind: ConfigMap
metadata:
  name: higress-https
  namespace: higress-system
data:
  cert: |
    automaticHttps: true
    renewBeforeDays: 30
    credentialConfig:
    - domains:
      - example.com
      tlsIssuer: letsencrypt
      tlsSecret: default-example-com-certificate
    - domains: 
      - sub.example.net
      tlsIssuer: aliyunssl
      tlsSecret: sub-example-net-tls-certificate
    - domains:  
       - statica.example.org
       - staticb.example.org
      tlsSecret: static-example-org-certificate
      cacertSecret: my-ca-cert
    - domains:  
       - "*"
      tlsSecret: default-certificate
    acmeIssuer:
    - email: your77658@yours69316.com
      name: letsencrypt
    - email: your77658@yours69316.com
      name:  aliyunssl
      sk: ""
      ak: ""
    version: "20240413090849"

在这个ConfigMap中,data字段中 cert 配置如下:

  1. automaticHttps: 设置为true,表示启用自动letsencrypt等自动签发功能。

  2. renewBeforeDays: 设置为30,表示在证书到期前30天自动续订证书。

  3. credentialConfig: 是一个列表,包含多个TLS证书的配置项。

    • domains: 指定了证书应该为哪些域名提供服务。
    • tlsIssuer: 指定了证书颁发机构(Certificate Authority,CA),如letsencrypt或aliyunssl。如何没有指定issuer,表示是静态配置
    • tlsSecret: 指定了存储TLS证书和密钥的Kubernetes Secret对象的名称。
    • cacertSecret: 指定了自定义CA证书的Kubernetes Secret对象的名称,用于当您使用自定义CA时。
  4. acmeIssuer

    • name: 这是ACME颁发机构的名称。有两个颁发机构:letsencrypt和aliyunssl, 目前只支持letsencrypt
    • email: 这是与ACME服务器注册账户时使用的电子邮件地址
    • ak: ACME访问密钥
    • sk: ACME账户私钥

Ⅱ. Does this pull request fix one issue?

  • Higress控制面集成能力
  • 控制Ingress最终生效
  • 支持域名上静态secret配置
  • 支持secret对接lets encrypt自动续签

Ⅲ. Why don't you add test cases (unit test/integration test)?

fixes #841
fixes #64

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@codecov-commenter
Copy link

codecov-commenter commented Mar 3, 2024

Codecov Report

Attention: Patch coverage is 18.06854% with 789 lines in your changes are missing coverage. Please review.

Project coverage is 36.24%. Comparing base (6c7b175) to head (6c4a4d9).
Report is 15 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #854      +/-   ##
==========================================
- Coverage   38.01%   36.24%   -1.78%     
==========================================
  Files          61       69       +8     
  Lines       10474    11464     +990     
==========================================
+ Hits         3982     4155     +173     
- Misses       6192     6993     +801     
- Partials      300      316      +16     
Files Coverage Δ
pkg/cmd/server.go 85.88% <100.00%> (+0.88%) ⬆️
pkg/ingress/kube/ingress/controller.go 62.28% <63.63%> (-0.07%) ⬇️
pkg/bootstrap/server.go 57.45% <60.00%> (-6.09%) ⬇️
pkg/ingress/kube/ingressv1/controller.go 5.36% <0.00%> (-0.06%) ⬇️
pkg/cert/util.go 8.16% <8.16%> (ø)
pkg/cert/secret.go 0.00% <0.00%> (ø)
pkg/cert/server.go 0.00% <0.00%> (ø)
pkg/cert/ingress.go 0.00% <0.00%> (ø)
pkg/cert/controller.go 0.00% <0.00%> (ø)
pkg/cert/storage.go 55.65% <55.65%> (ø)
... and 2 more

cmd/cert/main.go Outdated Show resolved Hide resolved
pkg/cert/config.go Show resolved Hide resolved
@2456868764 2456868764 force-pushed the feat-autohttps branch 2 times, most recently from 4c8e4b5 to a14dc76 Compare April 12, 2024 12:34
@2456868764
Copy link
Collaborator Author

已经重新提交,麻烦再看一下

@zzjin
Copy link
Contributor

zzjin commented Apr 15, 2024

这个configmap,有可能考虑改成CRD的模式吗,配置在一个configmap里面,不同的域名的RBAC会比较难控制

@johnlanni
Copy link
Collaborator

@zzjin 这个configmap我们不希望放到用户命名空间下,而是放在系统命名空间下(如:higress-system),统一交由集群SRE来管理

@johnlanni
Copy link
Collaborator

@2456868764 这个配置里的cert字段是否应该是higress的子字段比较合理

@2456868764
Copy link
Collaborator Author

higress-https

@2456868764 这个配置里的cert字段是否应该是higress的子字段比较合理

现在 cert 是保存新 higress-https configmap 里,是要保存到 higress-config configmap 里 higress 属性下?

1 similar comment
@2456868764
Copy link
Collaborator Author

higress-https

@2456868764 这个配置里的cert字段是否应该是higress的子字段比较合理

现在 cert 是保存新 higress-https configmap 里,是要保存到 higress-config configmap 里 higress 属性下?

@johnlanni
Copy link
Collaborator

higress-https

@2456868764 这个配置里的cert字段是否应该是higress的子字段比较合理

现在 cert 是保存新 higress-https configmap 里,是要保存到 higress-config configmap 里 higress 属性下?

哦,独立出来也有好处,可以用hgctl简化运维

Copy link
Collaborator

@johnlanni johnlanni left a comment

Choose a reason for hiding this comment

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

似乎没有看到在将ingress转换为istio gateway资源时,根据此份configmap配置credential的逻辑?

helm/core/values.yaml Outdated Show resolved Hide resolved
@johnlanni
Copy link
Collaborator

johnlanni commented Apr 16, 2024

似乎没有看到在将ingress转换为istio gateway资源时,根据此份configmap配置credential的逻辑?

secretName := extractTLSSecretName(rule.Host, ingressV1.TLS)
if secretName == "" {
// There no matching secret, so just skip.
continue
}
domainBuilder.Protocol = common.HTTPS
domainBuilder.SecretName = path.Join(c.options.ClusterId, cfg.Namespace, secretName)
// There is a matching secret and the gateway has already a tls secret.
// We should report the duplicated tls secret event.
if wrapperGateway.IsHTTPS() {
domainBuilder.Event = common.DuplicatedTls
domainBuilder.PreIngress = preDomainBuilder.Ingress
convertOptions.IngressDomainCache.Invalid = append(convertOptions.IngressDomainCache.Invalid,
domainBuilder.Build())
continue
}
// Append https server
wrapperGateway.Gateway.Servers = append(wrapperGateway.Gateway.Servers, &networking.Server{
Port: &networking.Port{
Number: uint32(c.options.GatewayHttpsPort),
Protocol: string(protocol.HTTPS),
Name: common.CreateConvertedName("https-"+strconv.FormatUint(uint64(c.options.GatewayHttpsPort), 10)+"-ingress", c.options.ClusterId),
},
Hosts: []string{rule.Host},
Tls: &networking.ServerTLSSettings{
Mode: networking.ServerTLSSettings_SIMPLE,
CredentialName: credentials.ToKubernetesIngressResource(c.options.RawClusterId, cfg.Namespace, secretName),
},
})

@2456868764 即这段逻辑需要调整适配

@2456868764
Copy link
Collaborator Author

似乎没有看到在将ingress转换为istio gateway资源时,根据此份configmap配置credential的逻辑?

secretName := extractTLSSecretName(rule.Host, ingressV1.TLS)
if secretName == "" {
// There no matching secret, so just skip.
continue
}
domainBuilder.Protocol = common.HTTPS
domainBuilder.SecretName = path.Join(c.options.ClusterId, cfg.Namespace, secretName)
// There is a matching secret and the gateway has already a tls secret.
// We should report the duplicated tls secret event.
if wrapperGateway.IsHTTPS() {
domainBuilder.Event = common.DuplicatedTls
domainBuilder.PreIngress = preDomainBuilder.Ingress
convertOptions.IngressDomainCache.Invalid = append(convertOptions.IngressDomainCache.Invalid,
domainBuilder.Build())
continue
}
// Append https server
wrapperGateway.Gateway.Servers = append(wrapperGateway.Gateway.Servers, &networking.Server{
Port: &networking.Port{
Number: uint32(c.options.GatewayHttpsPort),
Protocol: string(protocol.HTTPS),
Name: common.CreateConvertedName("https-"+strconv.FormatUint(uint64(c.options.GatewayHttpsPort), 10)+"-ingress", c.options.ClusterId),
},
Hosts: []string{rule.Host},
Tls: &networking.ServerTLSSettings{
Mode: networking.ServerTLSSettings_SIMPLE,
CredentialName: credentials.ToKubernetesIngressResource(c.options.RawClusterId, cfg.Namespace, secretName),
},
})

@2456868764 即这段逻辑需要调整适配

好的,我修改一下

@2456868764
Copy link
Collaborator Author

似乎没有看到在将ingress转换为istio gateway资源时,根据此份configmap配置credential的逻辑?

secretName := extractTLSSecretName(rule.Host, ingressV1.TLS)
if secretName == "" {
// There no matching secret, so just skip.
continue
}
domainBuilder.Protocol = common.HTTPS
domainBuilder.SecretName = path.Join(c.options.ClusterId, cfg.Namespace, secretName)
// There is a matching secret and the gateway has already a tls secret.
// We should report the duplicated tls secret event.
if wrapperGateway.IsHTTPS() {
domainBuilder.Event = common.DuplicatedTls
domainBuilder.PreIngress = preDomainBuilder.Ingress
convertOptions.IngressDomainCache.Invalid = append(convertOptions.IngressDomainCache.Invalid,
domainBuilder.Build())
continue
}
// Append https server
wrapperGateway.Gateway.Servers = append(wrapperGateway.Gateway.Servers, &networking.Server{
Port: &networking.Port{
Number: uint32(c.options.GatewayHttpsPort),
Protocol: string(protocol.HTTPS),
Name: common.CreateConvertedName("https-"+strconv.FormatUint(uint64(c.options.GatewayHttpsPort), 10)+"-ingress", c.options.ClusterId),
},
Hosts: []string{rule.Host},
Tls: &networking.ServerTLSSettings{
Mode: networking.ServerTLSSettings_SIMPLE,
CredentialName: credentials.ToKubernetesIngressResource(c.options.RawClusterId, cfg.Namespace, secretName),
},
})

@2456868764 即这段逻辑需要调整适配

@johnlanni 已经适配,麻烦看一下

Copy link
Collaborator

@johnlanni johnlanni left a comment

Choose a reason for hiding this comment

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

能否添加一个e2e test,可以只验证static secret的方式,校验服务端返回的证书与配置的是否一致

pkg/cert/config_test.go Show resolved Hide resolved
Copy link
Collaborator

@johnlanni johnlanni left a comment

Choose a reason for hiding this comment

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

现在区分了ingressv1和ingress两个目录对应不同的ingress版本,除了 ingressv1, ingress目录下的controller也需要调整一下

@2456868764
Copy link
Collaborator Author

现在区分了ingressv1和ingress两个目录对应不同的ingress版本,除了 ingressv1, ingress目录下的controller也需要调整一下

ingress & ingressv1 都修改,增加了 e2e test , 另外配置在 higress-https 里 secret 应该在 higress-system namespace 下?

Copy link
Collaborator

@johnlanni johnlanni left a comment

Choose a reason for hiding this comment

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

LGTM

@johnlanni johnlanni merged commit 03d2f01 into alibaba:main May 9, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants