ec_deployment: Add config to stateless resources #49
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Marc Lopez <[email protected]>
Signed-off-by: Marc Lopez <[email protected]>
Signed-off-by: Marc Lopez <[email protected]>
Signed-off-by: Marc Lopez <[email protected]>
Signed-off-by: Marc Lopez <[email protected]>
Signed-off-by: Marc Lopez <[email protected]>
karencfv
reviewed
Aug 24, 2020
Comment on lines
-112
to
-144
| "docker_image": { | ||
| Type: schema.TypeString, | ||
| Description: `A docker URI that allows overriding of the default docker image specified for this version`, | ||
| Optional: true, | ||
| }, | ||
|
|
||
| // APM System Settings | ||
| "debug_enabled": { | ||
| Type: schema.TypeBool, | ||
| Description: `Optionally enable debug mode for APM servers - defaults to false`, | ||
| Optional: true, | ||
| Default: false, | ||
| }, | ||
| "elasticsearch_password": { | ||
| Type: schema.TypeString, | ||
| Description: `Optionally override the account within APM - defaults to a system account that always exists (if specified, the username must also be specified)`, | ||
| Optional: true, | ||
| }, | ||
| "elasticsearch_url": { | ||
| Type: schema.TypeString, | ||
| Description: `Optionally override the URL to which to send data (for advanced users only, if unspecified the system selects an internal URL)`, | ||
| Optional: true, | ||
| }, | ||
| "elasticsearch_username": { | ||
| Type: schema.TypeString, | ||
| Description: `Optionally override the account within APM - defaults to a system account that always exists (if specified, the password must also be specified)`, | ||
| Optional: true, | ||
| }, | ||
| "kibana_url": { | ||
| Type: schema.TypeString, | ||
| Description: `Optionally override the URL to which to send data (for advanced users only, if unspecified the system selects an internal URL)`, | ||
| Optional: true, | ||
| }, |
There was a problem hiding this comment.
What happened to all of these fields? I assume the idea is to parse everything from raw JSON/YAML? just double checking
There was a problem hiding this comment.
These fields shouldn't be settable by API users and have the potential to cause breakage in the deployment, so removing them because they're not a good thing to have :D. I talked to Stack and Solutions about it and think that's the best course of action too.
Signed-off-by: Marc Lopez <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Adds a
configblock to all the stateless resources with the settablesettings being the same across the resources:
user_settings_yamluser_settings_overrides_yamluser_settings_jsonuser_settings_overrides_jsonExcept for:
secret_tokenanddebug_enabled.secret_session_keysecret_session_keyAll the fields above except
apm.debug_enabledmight be removed in thenear future pending an internal discussion.
The pull request has been split in different commits to ease the review
process, the commit names should be enough for the reviewers.
Related Issues
Resolves #22
Resolves #42
Resolves #6
How Has This Been Tested?
Types of Changes