Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Store resource identities in state (TF-23255) #36464

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

dbanck
Copy link
Member

@dbanck dbanck commented Feb 10, 2025

This PR implements the two new RPCs for resource identities and stores resource identities along with the resource attributes in state.

The new GetResourceIdentitySchemas RPC call is used to fetch all resource identity schemas for a provider. We fetch the identity schemas during the GetProviderSchema workflow and combine the results. In that way we end up with a resource type schema that contains everything we need later on:

type Schema struct {
Version int64
Body *configschema.Block
IdentityVersion int64
Identity *configschema.Object
}

Most methods, such as states.Decode and states.Encode, are updated to receive the above schema instead of a separate type and version.

The new UpgradeResourceIdentity RPC call is used to upgrade stored identities to the latest resource identity schema version. We only trigger the RPC if the stored identity schema version differs from the one in the resource identity schema.

Follow-up PRs

Target Release

1.12.x

CHANGELOG entry

  • This change is user-facing and I added a changelog entry.
  • This change is not user-facing.

@dbanck dbanck added the no-changelog-needed Add this to your PR if the change does not require a changelog entry label Feb 10, 2025
Copy link
Contributor

The equivalence tests failed. Please investigate here.

Copy link
Contributor

The equivalence tests failed. Please investigate here.

Copy link
Contributor

The equivalence tests failed. Please investigate here.

Copy link
Contributor

The equivalence tests failed. Please investigate here.

Copy link
Contributor

The equivalence tests failed. Please investigate here.

Copy link
Contributor

The equivalence tests failed. Please investigate here.

Copy link
Contributor

The equivalence tests failed. Please investigate here.

@dbanck dbanck force-pushed the dbanck/identity-state branch from 5754447 to 0c32d03 Compare March 5, 2025 13:49
@hashicorp hashicorp deleted a comment from github-actions bot Mar 5, 2025
@hashicorp hashicorp deleted a comment from github-actions bot Mar 5, 2025
@hashicorp hashicorp deleted a comment from github-actions bot Mar 5, 2025
@hashicorp hashicorp deleted a comment from github-actions bot Mar 5, 2025
@hashicorp hashicorp deleted a comment from github-actions bot Mar 5, 2025
@hashicorp hashicorp deleted a comment from github-actions bot Mar 5, 2025
@hashicorp hashicorp deleted a comment from github-actions bot Mar 5, 2025
@dbanck dbanck marked this pull request as ready for review March 6, 2025 15:30
@dbanck dbanck requested a review from a team as a code owner March 6, 2025 15:30
@@ -94,7 +117,7 @@ const (
// The returned object may share internal references with the receiver and
// so the caller must not mutate the receiver any further once once this
// method is called.
func (o *ResourceInstanceObject) Encode(ty cty.Type, schemaVersion uint64) (*ResourceInstanceObjectSrc, error) {
func (o *ResourceInstanceObject) Encode(schema providers.Schema) (*ResourceInstanceObjectSrc, error) {
Copy link
Member

Choose a reason for hiding this comment

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

something to come back to later might be to update the comments for the functions whose signature have changed

Copy link
Member

Choose a reason for hiding this comment

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

Actually, we should probably update them along with the change, or else we'll never find them again ;)

@@ -94,7 +117,7 @@ const (
// The returned object may share internal references with the receiver and
// so the caller must not mutate the receiver any further once once this
// method is called.
func (o *ResourceInstanceObject) Encode(ty cty.Type, schemaVersion uint64) (*ResourceInstanceObjectSrc, error) {
func (o *ResourceInstanceObject) Encode(schema providers.Schema) (*ResourceInstanceObjectSrc, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Actually, we should probably update them along with the change, or else we'll never find them again ;)

@@ -60,13 +63,13 @@ func (p *provider) GetSchema(_ context.Context, req *tfplugin5.GetProviderSchema

for typ, res := range p.schema.ResourceTypes {
resp.ResourceSchemas[typ] = &tfplugin5.Schema{
Version: res.Version,
Version: int64(res.Version),
Copy link
Member

Choose a reason for hiding this comment

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

The type here shouldn't have changed, does this need a conversion?


resp.IdentityTypes = make(map[string]providers.IdentitySchema)

protoResp, err := p.client.GetResourceIdentitySchemas(p.ctx, new(proto.GetResourceIdentitySchemas_Request))
Copy link
Member

Choose a reason for hiding this comment

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

We don't have a capability flag for this one, so we should be checking specifically for an unimplemented method here. I can't remember the best way to do this, but I thin the grpc framework will return codes.Unimplemented

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog-needed Add this to your PR if the change does not require a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants