cli: add --generate-config flag to constellation iam create command, which creates a config file with IAM values filled in

[msanft]

Proposed change(s)

• Add a generate-config flag to the constellation iam create command which enables automatic generation of a config file (path specified by the config flag. By default, constellation-conf.yaml) and automatic filling of the resulting IAM values into said file.

Checklist

• Update CHANGELOG.md
• Update docs
• Link to Milestone

[netlify]

✅ Deploy Preview for constellation-docs canceled.

Name	Link
🔨 Latest commit	0b12950
🔍 Latest deploy log	https://app.netlify.com/sites/constellation-docs/deploys/63bfdc11b4bf7300087d56ee

[msanft]

RFC
Currently, config.New() has no option to customize or disable config validation, which would be needed to use it to load an incomplete config (all IAM values & measurements will be missing at the point where i load it). Therefore i made a temporary function config.NewWithoutValidation() which skips the validation step. This is not a clean solution. I think we should allow an option to provide a custom validation function to config.New(). What do you think?

[3u13r]

Short summary from today's stand-up discussion:

• We don't want to write the IAM credentials/information directly into the constellation-conf.yaml but we want to reference a file (similar to what GCP already does)
• This way we you can safely share even Azure constellation-conf.yaml which currently include secrets
• This should side-step the requested change for moving the validation
• The field should be provider specific (aka. be .provider.azure.IAMConfig and not .IAMConfig)
• The new file will probably be one struct for which includes all fields for all CSPs which is then marshalled into the file.

[datosh]

Therefore i made a temporary function config.NewWithoutValidation() which skips the validation step. This is not a clean solution. I think we should allow an option to provide a custom validation function to config.New(). What do you think?

(Maybe it wont be necessary, but) I like the solution. The goal of config.New was to make it hard to use it wrong, e.g., for the to validate the config. NewWithoutValidation is explicit about what it is skipping. I think the API is good 🙂

What do you think is not clean about the solution or should be improved?

You could share more code between New and NewWithoutValidation so maintenance is easier. Maybe have a private new that implement most of the new logic.

[msanft]

What do you think is not clean about the solution or should be improved?

I think it might be better to keep the "working (at that point in time)" validation steps (such as checking if all fields are there and if the file is not corrupted generally) could be kept to prevent errors when the values are later on inserted into the file. But with the proposed rework, that would not be necessary i guess ;-)

[msanft]

We don't want to write the IAM credentials/information directly into the constellation-conf.yaml but we want to reference a file (similar to what GCP already does)

Since the values created by the iam create step are also used in other steps (e.g. create), we would also have to change all accesses to these values in the rest of the code. Since only 1 value of 1 cloud provider is sensitive here, i think we should rather look for another way. I might be missing a big point here, but from my thinking there are other ways to prevent the secret being exposed in the file.

This way we you can safely share even Azure constellation-conf.yaml which currently include secrets

We could also tackle this by using the environment variable CONSTELL_AZURE_CLIENT_SECRET_VALUE

[msanft]

Last consensus was (iirc):

• Leave constellation iam create as is, do not add config values automatically from there but only print them and ask the user to paste them in the config
• Create an opt-in for creating iam resources and filling iam values into the config automatically at the constellation config generate command

@katexochen are you OK with that?

[katexochen]

@katexochen are you OK with that?

Yes, sounds good! :)

[msanft]

Any1 got an idea on how to implement this in a "clean" manner?

I think adding all IAM-flags for every CP to config generate would be ugly and redundant as we have them on iam create already. But how could we get around it? Have subcommands for config generate? Seems ugly as well.

Any ideas are welcome 😄

[katexochen]

Create an opt-in for creating iam resources and filling iam values into the config automatically at the constellation config generate command

Iirc Thomas and I were also okay with adding an opt-in option to iam command to generate config instead, but you didn't like it this way? Would solve the problem imo...

[msanft]

Create an opt-in for creating iam resources and filling iam values into the config automatically at the constellation config generate command

Iirc Thomas and I were also okay with adding an opt-in option to iam command to generate config instead, but you didn't like it this way? Would solve the problem imo...

Sounds good. Maybe I mixed something up, I thought you were against doing it that way. Then I'll go work on that! Thank you :)

[katexochen]

I thought you were against doing it that way

I was just against doing that opt-out.

[datosh]

Yes, the chosen approach works for me as well!

I have built & tested the current state locally for Azure. I have two comments, but they are unrelated to this PR (so we can choose to fix this here or in another PR):

The output when creating resources for Azure is misaligned. I think it would be more readable to have a consistent indentation for region, resource group & service principal:

./constellation iam create azure --region westus --resourceGroup fabian-delete-me-iam --servicePrincipal fabian-delete-me-iam --generate-config
The following IAM configuration will be created:
Region: westus
Resource Group: fabian-delete-me-iam
Service Principal:      fabian-delete-me-iam
The configuration file constellation-conf.yaml will be automatically generated and populated with the IAM values.
Do you want to create the configuration? [y/n]: y
Creating  
Your IAM configuration was created and filled into constellation-conf.yaml successfully.

The help text for resource group is not correct:

--resourceGroup string      Name of the resource group your IAM resources will be created in.

When I run iam create two resource groups seem to be created. The one I provided and an additional with suffix -identity. The suffixed one contains the IAM resources mentioned in helptext. I think we should mention both in helptext.

[katexochen]

Only some minor architecture and language comments, idea looks good so far.

[katexochen]

lgtm, well done. 🐳

Some minor follow ups that shouldn't be done in this PR:

• The IAM commands currently lack debug logging, we should add some lines, at least the parsed flags etc
• Look if we can reducing redundancy in the IAM commands, maybe by introducing an interface that the the flags fullfill, so that that the core logic doesn't have to be duplicated

[msanft]

The IAM commands currently lack debug logging, we should add some lines, at least the parsed flags etc

I forgot to mention this in our conversation today. I only discovered this today, think we had a bad timing between merging IAM and Debug logging so that they "missed" each other.

Look if we can reducing redundancy in the IAM commands, maybe by introducing an interface that the the flags fullfill, so that that the core logic doesn't have to be duplicated

Agreed.

I will bring up both points in the next backlog refinement.

[katexochen]

I will bring up both points in the next backlog refinement.

Feel free to already create tickets for this and start implementation in a separate PR as you like.

[msanft]

@katexochen am I missing something or could we omit the second "if"-clause here? Just noticed it

constellation/cli/internal/cmd/configgenerate.go

Lines 67 to 88 in 372e4b9

	if flags.file == "-" {
	content, err := encoder.NewEncoder(conf).Encode()
	if err != nil {
	return fmt.Errorf("encoding config content: %w", err)
	}
	cg.log.Debugf("Writing YAML data to stdout")
	_, err = cmd.OutOrStdout().Write(content)
	return err
	}
	cg.log.Debugf("Writing YAML data to configuration file")
	if err := fileHandler.WriteYAML(flags.file, conf, file.OptMkdirAll); err != nil {
	return err
	}
	if flags.file != "-" {
	cmd.Println("Config file written to", flags.file)
	cmd.Println("Please fill in your CSP-specific configuration before proceeding.")
	cmd.Println("For more information refer to the documentation:")
	cmd.Println("\thttps://docs.edgeless.systems/constellation/getting-started/first-steps")
	}