Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion dracut/30ignition/ignition-fetch.service
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,4 @@ After=network.target
Type=oneshot
RemainAfterExit=yes
EnvironmentFile=/run/ignition.env
ExecStart=/usr/bin/ignition --root=/sysroot --platform=${PLATFORM_ID} --stage=fetch ${IGNITION_ARGS}
ExecStart=/usr/bin/ignition --root=/sysroot --platform=${PLATFORM_ID} --stage=fetch --generate-cloud-config=true ${IGNITION_ARGS}

Choose a reason for hiding this comment

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

medium

This change hardcodes --generate-cloud-config=true for the fetch stage. As noted in the RFC description, this is "Option B" which auto-enables the feature. While this simplifies the default case, it removes the flexibility for an Azure user to provide a standard Ignition config via user data if they wished. Have you considered making this configurable via a kernel argument, allowing users to opt-out of this new behavior if needed? This would align more with "Option A" and provide greater control.

1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ require (
cloud.google.com/go/storage v1.58.0
github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.13.1
github.com/Azure/azure-sdk-for-go/sdk/storage/azblob v1.6.3
github.com/GehirnInc/crypt v0.0.0-20230320061759-8cc1b52080c5
github.com/aws/aws-sdk-go-v2 v1.41.0
github.com/aws/aws-sdk-go-v2/credentials v1.19.6
github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.18.16
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ github.com/AzureAD/microsoft-authentication-extensions-for-go/cache v0.1.1 h1:WJ
github.com/AzureAD/microsoft-authentication-extensions-for-go/cache v0.1.1/go.mod h1:tCcJZ0uHAmvjsVYzEFivsRTN00oz5BEsRgQHu5JZ9WE=
github.com/AzureAD/microsoft-authentication-library-for-go v1.6.0 h1:XRzhVemXdgvJqCH0sFfrBUTnUJSBrBf7++ypk+twtRs=
github.com/AzureAD/microsoft-authentication-library-for-go v1.6.0/go.mod h1:HKpQxkWaGLJ+D/5H8QRpyQXA1eKjxkFlOMwck5+33Jk=
github.com/GehirnInc/crypt v0.0.0-20230320061759-8cc1b52080c5 h1:IEjq88XO4PuBDcvmjQJcQGg+w+UaafSy8G5Kcb5tBhI=
github.com/GehirnInc/crypt v0.0.0-20230320061759-8cc1b52080c5/go.mod h1:exZ0C/1emQJAw5tHOaUDyY1ycttqBAPcxuzf7QbY6ec=
github.com/GoogleCloudPlatform/opentelemetry-operations-go/detectors/gcp v1.30.0 h1:sBEjpZlNHzK1voKq9695PJSX2o5NEXl7/OL3coiIY0c=
github.com/GoogleCloudPlatform/opentelemetry-operations-go/detectors/gcp v1.30.0/go.mod h1:P4WPRUkOhJC13W//jWpyfJNDAIpvRbAUIYLX/4jtlE0=
github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/metric v0.54.0 h1:lhhYARPUu3LmHysQ/igznQphfzynnqI3D75oUyw1HXk=
Expand Down
43 changes: 35 additions & 8 deletions internal/exec/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,15 @@ var (

// Engine represents the entity that fetches and executes a configuration.
type Engine struct {
ConfigCache string
FetchTimeout time.Duration
Logger *log.Logger
NeedNet string
Root string
PlatformConfig platform.Config
Fetcher *resource.Fetcher
State *state.State
ConfigCache string
FetchTimeout time.Duration
GenerateCloudConfig bool
Logger *log.Logger
NeedNet string
Root string
PlatformConfig platform.Config
Fetcher *resource.Fetcher
State *state.State
}

// Run executes the stage of the given name. It returns true if the stage
Expand Down Expand Up @@ -286,6 +287,10 @@ func (e *Engine) acquireProviderConfig() (cfg types.Config, err error) {
// is unavailable. This will also render the config (see renderConfig) before
// returning.
func (e *Engine) fetchProviderConfig() (types.Config, error) {
if e.GenerateCloudConfig {
return e.fetchGeneratedConfig()
}

platformConfigs := []platform.Config{
cmdline.Config,
system.Config,
Expand Down Expand Up @@ -331,6 +336,28 @@ func (e *Engine) fetchProviderConfig() (types.Config, error) {
return configFetcher.RenderConfig(cfg)
}

func (e *Engine) fetchGeneratedConfig() (types.Config, error) {
e.Logger.Info("using generated cloud config for platform %q", e.PlatformConfig.Name())
cfg, err := e.PlatformConfig.GenerateConfig(e.Fetcher)
if err != nil {
return types.Config{}, err
}

e.State.FetchedConfigs = append(e.State.FetchedConfigs, state.FetchedConfig{
Kind: "user",
Source: fmt.Sprintf("%s-generator", e.PlatformConfig.Name()),
Referenced: false,
})

configFetcher := ConfigFetcher{
Logger: e.Logger,
Fetcher: e.Fetcher,
State: e.State,
}

return configFetcher.RenderConfig(cfg)
}
Comment on lines +339 to +359

Choose a reason for hiding this comment

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

medium

This new fetchGeneratedConfig function is missing a call to e.Fetcher.UpdateHttpTimeoutsAndCAs before configFetcher.RenderConfig is called. The original fetchProviderConfig flow includes this step to ensure that any timeouts or CAs specified in the just-fetched config are used for subsequent resource fetches (e.g., remote files referenced in the config). Without this, Ignition will use the default timeouts, which might not be what the user intended.

Additionally, there is significant code duplication between fetchGeneratedConfig and the end of fetchProviderConfig. Consider refactoring to have a single ConfigFetcher creation and RenderConfig call to improve maintainability.

func (e *Engine) fetchGeneratedConfig() (types.Config, error) {
	e.Logger.Info("using generated cloud config for platform %q", e.PlatformConfig.Name())
	cfg, err := e.PlatformConfig.GenerateConfig(e.Fetcher)
	if err != nil {
		return types.Config{}, err
	}

	e.State.FetchedConfigs = append(e.State.FetchedConfigs, state.FetchedConfig{
		Kind:       "user",
		Source:     fmt.Sprintf("%s-generator", e.PlatformConfig.Name()),
		Referenced: false,
	})

	// Update the http client to use the timeouts and CAs from the newly fetched
	// config, before rendering and fetching remote resources.
	if err := e.Fetcher.UpdateHttpTimeoutsAndCAs(cfg.Ignition.Timeouts, cfg.Ignition.Security.TLS.CertificateAuthorities, cfg.Ignition.Proxy); err != nil {
		return types.Config{}, err
	}

	configFetcher := ConfigFetcher{
		Logger:  e.Logger,
		Fetcher: e.Fetcher,
		State:   e.State,
	}

	return configFetcher.RenderConfig(cfg)
}


func (e *Engine) signalNeedNet() error {
if err := executil.MkdirForFile(e.NeedNet); err != nil {
return err
Expand Down
44 changes: 27 additions & 17 deletions internal/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,20 +48,22 @@ func main() {

func ignitionMain() {
flags := struct {
configCache string
fetchTimeout time.Duration
needNet string
platform platform.Name
root string
stage stages.Name
stateFile string
version bool
logToStdout bool
configCache string
fetchTimeout time.Duration
generateCloudConfig bool
needNet string
platform platform.Name
root string
stage stages.Name
stateFile string
version bool
logToStdout bool
}{}

flag.StringVar(&flags.configCache, "config-cache", "/run/ignition.json", "where to cache the config")
flag.DurationVar(&flags.fetchTimeout, "fetch-timeout", exec.DefaultFetchTimeout, "initial duration for which to wait for config")
flag.StringVar(&flags.needNet, "neednet", "/run/ignition/neednet", "flag file to write from fetch-offline if networking is needed")
flag.BoolVar(&flags.generateCloudConfig, "generate-cloud-config", false, "generate config from cloud provider metadata instead of fetching")
flag.Var(&flags.platform, "platform", fmt.Sprintf("current platform. %v", platform.Names()))
flag.StringVar(&flags.root, "root", "/", "root of the filesystem")
flag.Var(&flags.stage, "stage", fmt.Sprintf("execution stage. %v", stages.Names()))
Expand All @@ -71,6 +73,11 @@ func ignitionMain() {

flag.Parse()

// Never allow cloud config generation during fetch-offline stage (no networking)
if flags.stage == "fetch" && flags.platform == "azure" {
flags.generateCloudConfig = true
}
Comment on lines +76 to +79

Choose a reason for hiding this comment

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

high

The comment on line 76 is misleading. It states "Never allow cloud config generation during fetch-offline stage", but the code block enables generateCloudConfig for the fetch stage on Azure. This can cause confusion for future developers. The comment should be updated to accurately reflect that this block auto-enables the feature for the Azure platform.

Suggested change
// Never allow cloud config generation during fetch-offline stage (no networking)
if flags.stage == "fetch" && flags.platform == "azure" {
flags.generateCloudConfig = true
}
// Auto-enable cloud config generation for Azure during the fetch stage.
// This is part of the "Option B" approach described in the RFC.
if flags.stage == "fetch" && flags.platform == "azure" {
flags.generateCloudConfig = true
}


if flags.version {
fmt.Printf("%s\n", version.String)
return
Expand All @@ -91,6 +98,8 @@ func ignitionMain() {

logger.Info("%s", version.String)
logger.Info("Stage: %v", flags.stage)
logger.Info("Platform: %v", flags.platform)
logger.Info("GenerateCloudConfig: %v", flags.generateCloudConfig)

platformConfig := platform.MustGet(flags.platform.String())
fetcher, err := platformConfig.NewFetcher(&logger)
Expand All @@ -104,14 +113,15 @@ func ignitionMain() {
os.Exit(3)
}
engine := exec.Engine{
Root: flags.root,
FetchTimeout: flags.fetchTimeout,
Logger: &logger,
NeedNet: flags.needNet,
ConfigCache: flags.configCache,
PlatformConfig: platformConfig,
Fetcher: &fetcher,
State: &state,
Root: flags.root,
FetchTimeout: flags.fetchTimeout,
GenerateCloudConfig: flags.generateCloudConfig,
Logger: &logger,
NeedNet: flags.needNet,
ConfigCache: flags.configCache,
PlatformConfig: platformConfig,
Fetcher: &fetcher,
State: &state,
}

err = engine.Run(flags.stage.String())
Expand Down
10 changes: 10 additions & 0 deletions internal/platform/platform.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ type Provider struct {
Status func(stageName string, f resource.Fetcher, e error) error
DelConfig func(f *resource.Fetcher) error

// Generates a platform-specific Ignition config from cloud provider metadata.
GenerateCloudConfig func(f *resource.Fetcher) (types.Config, error)

// Fetch, and also save output files to be written during files stage.
// Avoid, unless you're certain you need it.
FetchWithFiles func(f *resource.Fetcher) ([]types.File, types.Config, report.Report, error)
Expand Down Expand Up @@ -104,6 +107,13 @@ func (c Config) DelConfig(f *resource.Fetcher) error {
}
}

func (c Config) GenerateConfig(f *resource.Fetcher) (types.Config, error) {
if c.p.GenerateCloudConfig != nil {
return c.p.GenerateCloudConfig(f)
}
return types.Config{}, ErrNoProvider
}

var configs = registry.Create("platform configs")

func Register(provider Provider) {
Expand Down
Loading