Skip to content

cubemaster: support dns flag for template image creation#98

Merged
ls-ggg merged 1 commit into
TencentCloud:masterfrom
YangYuS8:fix-issue-12-dns-flag
Apr 27, 2026
Merged

cubemaster: support dns flag for template image creation#98
ls-ggg merged 1 commit into
TencentCloud:masterfrom
YangYuS8:fix-issue-12-dns-flag

Conversation

@YangYuS8
Copy link
Copy Markdown
Contributor

Summary

  • add repeatable --dns support to cubemastercli tpl create-from-image
  • propagate DNS server overrides into generated template container specs
  • include DNS configuration in the template spec fingerprint

Problem

cubemastercli tpl create-from-image previously had no way to specify DNS nameservers, even though the underlying container model already supports DnsConfig.

This makes it difficult to create templates for environments that require custom DNS resolution, such as private networks or internal service discovery.

Fix

  • add DnsConfig to ContainerOverrides
  • parse repeated --dns values in the create-from-image CLI flow
  • apply DNS server overrides when generating the template create request
  • include DNS config in fingerprint generation so different DNS settings produce different template artifacts

Example:

cubemastercli tpl create-from-image \
  --image ccr.ccs.tencentyun.com/ags-image/sandbox-code:latest \
  --dns 8.8.8.8 \
  --dns 1.1.1.1

Closes #12

Validation

  • cd CubeMaster && go test ./cmd/cubemastercli/commands/cubebox -run DNS -count=1
  • cd CubeMaster && go test ./pkg/templatecenter -run DNS -count=1

Notes

  • This PR only adds nameserver support via --dns.
  • It intentionally does not add --dns-search or --dns-option to keep the scope focused.

@fslongjin
Copy link
Copy Markdown
Member

/cubebot review

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 27, 2026

Claude finished @fslongjin's task in 2m 46s —— View job


PR Review: DNS Flag for Template Image Creation

  • Gather context and read changed files
  • Understand the implementation
  • Provide detailed review feedback

Overall: Clean, well-scoped PR that follows existing patterns. The implementation is correct and consistent with how other override fields (probe, rlimit, env) are handled. A few suggestions below.


Issues

1. No DNS server input validation

The --dns flag accepts any string without validating it's a valid IP address. A user could pass --dns not-an-ip and it would be silently accepted and propagated to the container spec, likely causing runtime DNS resolution failures.

CubeMaster/cmd/cubemastercli/commands/cubebox/template.go:1296-1298

if len(dnsServers) > 0 {
    overrides.DnsConfig = &types.DNSConfig{Servers: dnsServers}
}

Consider adding IP validation (e.g., net.ParseIP) in parseContainerOverrides, similar to how Docker validates --dns values. This is especially important since this is a CLI entry point where user input should be validated.

Fix this →

2. Fingerprint test should also cover DNS vs no-DNS

TestBuildTemplateSpecFingerprintUsesDNSConfig only verifies that two different DNS configs produce different fingerprints. The more meaningful case is that a request with DNS config produces a different fingerprint than one without. This is the scenario that would prevent incorrect template reuse.

CubeMaster/pkg/templatecenter/template_image_test.go:202-228

Fix this →

3. Missing test: no --dns flag leaves DnsConfig nil

There's no test verifying that when --dns is not provided, DnsConfig remains nil on the resulting overrides. This is important to prevent regressions where the nil-check in dnsConfigOrNil might be bypassed.

CubeMaster/cmd/cubemastercli/commands/cubebox/template_test.go

Fix this →


Nit

4. dnsConfigOrNil vs inline access

dnsConfigOrNil is consistent with the existing probeOrNil pattern, so this is a style point rather than a problem. However, the function is trivial — it just checks overrides == nil then returns overrides.DnsConfig, which Go handles safely via nil pointer dereference on the field access. If you ever need to transform the value (e.g., deep copy like cloneCubeVSContext), a helper makes more sense. As-is, req.ContainerOverrides.DnsConfig with a preceding nil check at the call site would be equally clear. No action needed since consistency with probeOrNil is a valid justification.


What looks good

  • Fingerprint inclusion works correctly for free because ContainerOverrides (now containing DnsConfig) is already serialized in buildTemplateSpecFingerprint. The test properly validates this.
  • The "return nil overrides when nothing is set" check at line 1273 is correctly updated to include len(dnsServers) == 0.
  • The Container struct already had a DnsConfig field (line 150 of types.go), so the types are naturally compatible — no adapter needed.
  • Test coverage spans all three layers: CLI parsing, fingerprint generation, and request generation.

Signed-off-by: YangYuS8 <YangYuS8@163.com>
@YangYuS8 YangYuS8 force-pushed the fix-issue-12-dns-flag branch from f7befb4 to 227a4c1 Compare April 27, 2026 05:05
@YangYuS8
Copy link
Copy Markdown
Contributor Author

Thanks for the review. I addressed the actionable suggestions:

  • added IP validation for repeated "--dns" values using "net.ParseIP"
  • added tests for invalid DNS input and the no-DNS case
  • expanded the fingerprint test to cover DNS vs no-DNS
  • kept "dnsConfigOrNil" unchanged for consistency with the existing helper pattern

@ls-ggg ls-ggg merged commit 147657e into TencentCloud:master Apr 27, 2026
6 checks passed
@YangYuS8 YangYuS8 deleted the fix-issue-12-dns-flag branch April 27, 2026 07:41
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.

[Feature Request] No DNS configuration support when creating templates via cubemastercli

3 participants