Live pulumi state #957
Live pulumi state #957
Conversation
| @@ -16,3 +16,7 @@ function properties( | |||
| Name: object.apply((o) => o.name), | |||
| } | |||
| } | |||
|
|
|||
| function importResource(args: Args): pulumi.Output<pulumi.UnwrappedObject<aws.GetRegionResult>> { | |||
There was a problem hiding this comment.
Just curious, what's the point of having this when it's the same as create?
There was a problem hiding this comment.
just because the code right now only looks for the importResource function and i didnt want to modify too much code since this was exploratory
pkg/infra/cli.go
Outdated
|
|
||
| getLiveStateCmd := &cobra.Command{ | ||
| Use: "GetLiveState", | ||
| Short: "Get the live state for a given graph and state file", |
There was a problem hiding this comment.
nit: it's not clear to me what the output/purpose of this is from the description. The pulumi state file is the live state, no? This is more like "fill in the input graph properties from the live state", right?
There was a problem hiding this comment.
yeah i can update
| for _, resource := range pulumiState { | ||
| mapping, ok := p.templates[resource.Type] | ||
| if !ok { | ||
| zap.S().Warnf("no mapping found for resource type %s", resource.Type) |
There was a problem hiding this comment.
Warn here seems odd, maybe debug? If the resource isn't needed, then it's hardly a warning that we don't support it.
There was a problem hiding this comment.
i guess we dont know if its not needed, i was thinking warning because my eventualy goal would be to error, but i guess well never support all resources. Still feels like its something for the user to be aware of
There was a problem hiding this comment.
Hm, maybe I misunderstood how this works then. I thought the input graph would be used for which things connect to other things, then the reading the state would fill out the properties (like id). In that scenario, I'd expect there to be an error if a resource in the input graph doesn't have a template. Sounds like that's not how that works. Can you explain what the input graph is for then? I could probably figure it out by looking closer at the code, but if you could give a quick description that'd help.
| construct.ResourceId, | ||
| construct.Properties, |
There was a problem hiding this comment.
nit: this is pretty much just a *contruct.Resource
| } | ||
| } | ||
| // Convert the keys to camel case | ||
| return id, convertKeysToCamelCase(properties), nil |
There was a problem hiding this comment.
TODO - this should probably come from the templates somehow - not sure if camel case is universally applicable (eg tag keys).
There was a problem hiding this comment.
this is in the pulumi file itself, i agree it will need to be more granular in the future though because not all things are converted via camelCase. I also dont know how the nested objects will work yet, thats where this could have issues
| } | ||
| ) | ||
|
|
||
| func (p pulumiStateConverter) ConvertState(data []byte) (State, error) { |
There was a problem hiding this comment.
nit: generally prefer APIs which don't require allocation, so a io.Reader (use json.NewEncoder(reader).Decode)
| for k, v := range resource.Outputs { | ||
| if mapping, ok := template.PropertyMappings[k]; ok { | ||
| if strings.Contains(mapping, "#") { | ||
| // TODO: Determine how to cross correlate references/resource properties |
There was a problem hiding this comment.
nit: can you give an example of what the v looks like when the mapping is a ref? And maybe add a test case with the expected correct result commented out?
There was a problem hiding this comment.
theres a test case to just make sure we ignore these for now because we dont need to process them yet, but i added an example comment
| @@ -66,7 +67,8 @@ func Test_pulumiStateConverter_ConvertState(t *testing.T) { | |||
| p := pulumiStateConverter{ | |||
| templates: tt.templates, | |||
| } | |||
| got, err := p.ConvertState(tt.data) | |||
| reader := bytes.NewReader(tt.data) | |||
There was a problem hiding this comment.
very nit: could change data to be a string then use strings.NewReader
| @@ -27,7 +29,8 @@ func NewPulumiReader(templates map[string]statetemplate.StateTemplate, kb knowle | |||
|
|
|||
| func (p stateReader) ReadState(state []byte, graph construct.Graph) (construct.Graph, error) { | |||
| returnGraph := construct.NewGraph() | |||
| internalState, err := p.converter.ConvertState(state) | |||
| stateReader := bytes.NewReader(state) | |||
There was a problem hiding this comment.
like for the converter, the reader could also use io.Reader so that the File passes all the way down
There was a problem hiding this comment.
oh i see, sure let me change that
jhsinger-klotho
force-pushed
the
live_pulumi_state
branch
from
e9e830d to
e791104
Compare
| @@ -128,8 +129,8 @@ func GetLiveState(cmd *cobra.Command, args []string) error { | |||
| return err | |||
| } | |||
| } | |||
|
|
|||
| result, err := reader.ReadState(stateBytes, input.Graph) | |||
| bytesReader := bytes.NewReader(stateBytes) | |||
There was a problem hiding this comment.
Instead of ReadFile earlier, I mean using os.Open. Sorry for the back-and-forth, I could've explained it better
| @@ -67,7 +67,8 @@ func (p pulumiStateConverter) convertResource(resource Resource, template statet | |||
| for k, v := range resource.Outputs { | |||
| if mapping, ok := template.PropertyMappings[k]; ok { | |||
| if strings.Contains(mapping, "#") { | |||
| // TODO: Determine how to cross correlate references/resource properties | |||
| // TODO: Determine how to cross correlate references/resource properties. | |||
| // an example of this is the subnets vpcId field (value = vpc-123456789), to where internally its modeld as a "resource". | |||
start of being able to read in the pulumi state file to correlate arns to resources
Standard checks