-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Support config domain URL for cloudcore #2126
Conversation
Removed irrelevant labels. |
@@ -39,7 +39,8 @@ func SignCerts() ([]byte, []byte, error) { | |||
Organization: []string{"KubeEdge"}, | |||
Usages: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, | |||
AltNames: certutil.AltNames{ | |||
IPs: getIps(hubconfig.Config.AdvertiseAddress), | |||
DNSNames: hubconfig.Config.DNSNames, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: DNSNames => DomainNames ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: DNSNames => DomainNames ?
@kevin-wangzefeng Thank for your suggestion. DNSNames is a bit weird, but in k8s, it is named like this, so we'd better keep using DNSNames.
@@ -63,6 +64,10 @@ func (cu *KubeCloudInstTool) InstallTools() error { | |||
cloudCoreConfig.Modules.CloudHub.AdvertiseAddress = strings.Split(cu.AdvertiseAddress, ",") | |||
} | |||
|
|||
if cu.DNSName != "" { | |||
cloudCoreConfig.Modules.CloudHub.DNSNames = strings.Split(cu.DNSName, ",") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need validations somewhere on the user input
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kevin-wangzefeng sure, I will be create a new pr to do it.
/milestone v1.5 |
@@ -28,6 +28,7 @@ type InitOptions struct { | |||
KubeConfig string | |||
Master string | |||
AdvertiseAddress string | |||
DNS string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the keadm
command has the DNS
flag option?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fisherxu yes, in another pr.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can add it to this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fisherxu ok, i will merge them later.
ab98a5e
to
f8beffe
Compare
@@ -97,6 +97,9 @@ func addJoinOtherFlags(cmd *cobra.Command, initOpts *types.InitOptions) { | |||
|
|||
cmd.Flags().StringVar(&initOpts.AdvertiseAddress, types.AdvertiseAddress, initOpts.AdvertiseAddress, | |||
"Use this key to set SANs in certificate of cloudcore. eg: 10.10.102.78,10.10.102.79") | |||
|
|||
cmd.Flags().StringVar(&initOpts.DNS, types.DomainName, initOpts.DNS, | |||
"Use this key to set SANs in certificate of cloudcore. eg: www.cloudcore.cn,www.kubeedge.cn") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Use this key to set SANs in certificate of cloudcore. eg: www.cloudcore.cn,www.kubeedge.cn") | |
"Use this key to set domain names in cloudcore's certificate SubAltNames field. eg: www.cloudcore.cn,www.kubeedge.cn") |
Also update the ip field above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fisherxu ok
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fisherxu The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #1642
Special notes for your reviewer:
Does this PR introduce a user-facing change?: