Proposal for creating a separate "tracking issue" for each accepted RFC,
which represents the implementation of the features described in that RFC
separately from the potentially-many feature request issues it aims to
address.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
Unlike the other nesting modes, we need to use some quite different code
for comparing object-backed vs. map-backed blocks, which are both possible
interpretations of the NestingMap mode depending on whether the inner
block type has any dynamically-typed attributes.
Therefore we split that case into two parts depending on what type kind
we find, so that each of the parts can then be shaped more like the other
type-specific variants of assertNestedBlockCompatible. (This also removes
one level of if nesting to placate the nestif linter.)
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
This function starts with a general part that deals with conditions that
are common to all types, but then dispatches into different codepaths
depending on the type kind.
To keep the main function shorter, here we decompose the type-kind-specific
handling into separate functions, making assertValueCompatible now end
with a simpler dispatch table.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
The main function is now just a jump table into a separate function for
each nesting mode. The observable behavior is unchanged.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
This splits out the handling of individual attributes and individual nested
block types into separate functions, thereby reducing the length and
complexity of the top-level function.
As of this commit, assertNestedBlockCompatible is still too long to pass
our current function length linting limit, but we'll address that in a
later commit to avoid changing too much at once.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
Passing the result of fmt.Sprintf as the format string to path.NewErrorf
is redundant. It can also potentially cause problems if the result would
also contain formatting verbs, although in this case the input is under
this function's full control so this was just a waste of time rather than
a behavior problem.
Go error strings also conventionally start with lowercase letters and act
as sentence fragments rather than full sentences, so the prefix used for
a zero-length path is now "root object" instead of "Root object".
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
Signed-off-by: Mikel Olasagasti Uranga <mikel@olasagasti.info>
Signed-off-by: James Humphries <james@james-humphries.co.uk>
Co-authored-by: Mikel Olasagasti Uranga <mikel@olasagasti.info>
This commit adds the definitions of provider protocol 5.6, 5.7, 6.6, and
6.7 to our archive of the historical protocol versions and then adopts
5.7 as the current version of major version 5 and 6.7 as the current
version of major version 6.
These MPL-licensed schema definitions are from the plugin protocol server
implementation in this repository, copyright HashiCorp:
https://github.com/hashicorp/terraform-plugin-go
The only modifications made are to change the "option go_package" directive
to match where the stubs need to be generated for OpenTofu, and to claim
copyright for that change and thus make the copyright header consistent
with what our pre-commit rules require.
The regeneration of the Go API stubs for the two major protocol versions
introduces some new fields and messages that OpenTofu does not yet support
but will happily ignore. Future work might make some use of these new
additions, but that's out of scope of this change that is intended only
to synchronize our protocol definition with what new plugin server releases
are likely to be linked against.
This commit continues the existing precedent of having the stubs for the
newly-added interface methods in package grpcwrap being just a panicking
stub, which is how they will remain until a future project begins using
those methods in a way which requires them to be implemented, since
implementing the wrappers would require a deeper understanding of the
desired behavior of those methods.
It appears that we previously accepted a pull request to correct a typo
that originated in the older versions of the upstream protocol definitions,
but I have intentionally not forward-ported that here because it seems
clearer to keep these definitions as close as possible to their source
of truth from upstream, given that our current intention is to follow the
protocol as documented and not to change it.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
Signed-off-by: Jon Johnson <jon.johnson@chainguard.dev>
Signed-off-by: Jon Johnson <jonjohnsonjr@gmail.com>
Co-authored-by: Oleksandr Levchenkov <ollevche@gmail.com>
This function was previously failing the nestif lint rule. Factoring the
loop body out into a separate function avoids that problem and also
subjectively makes this easier to follow by separating the functional-style
enhanceFunctionDiag from the mutation of the backing array of the caller's
diags slice.
This also transforms enhanceFunctionDiags into a package-level function
rather than a method of Scope, since in practice it doesn't actually use
anything from Scope.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
Whichever function were responsible for instantiating this map would
inevitably end up scaling in length based on the number of funtions and
thus violate the function length linter.
As a compromise, we'll factor out the function table into its own function
that is exempt from that linter but focused only on constructing the base
function table, and then Scope.Functions can be shorter by dealing only
with the scope-specific customizations of the returned table.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
This is an assortment of small changes to fix all of the remaining lint
failures in this package, based on our current golangci-lint configuration.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
This function was too long for our function length linting rule, so we'll
split each of the type kinds with special handling into their own function
and thus the main typeString function is just a straightforward dispatch
table with only one statement per case.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
This value was too long for our function length lint rule, and factoring
out the printing of null values makes this more balanced with how we're
already handling unknown values and sensitive values so that the main
body of FormatValue is focused on the normal value printing case.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
The functions in these files are for handling older state snapshot formats
that current OpenTofu versions never generate, and so it's highly unlikely
that we'll ever make substantial changes to these functions.
Therefore it's unjustified to risk reworking it to pass linting rules, and
so we'll add nolint comments instead. Our priority is to make as few
changes as possible to these functions, to minimize the risk of regressing
a upgrade paths that are exercised very infrequently.
(For context, state version 4 has been current ever since Terraform
v0.12.0, and so the earlier versions are long obsolete.)
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
These packages are frozen copies of old code from much older versions of
the product that are preserved to keep the state storage backends working
until we decide on a way to get them out of this codebase entirely.
Therefore the only potential future change to this code is to delete it
once it's no longer needed. It would not be worth the risk or time
investment to rework these to meet our strict complexity linting rules.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
The code in this package is all snapshot from the Go codebase in older
versions, inlined here to allow OpenTofu's cidr-calculation-related
functions to preserve their original behavior despite upstream changing
the parsing rules in a breaking way.
This code is intentionally modified as little as possible from the upstream
code it was derived from. We are imposing on ourselves considerably
stricter style conventions than the Go project follows and so we need
to disable various linters for this package to allow this code to remain
written in the Go idiomatic style, rather than in OpenTofu's stricter
local style.
In particular, we've chosen to prohibit ourselves from using named return
values or package-global variables, despite those both being typical in the
standard library and in other codebases.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
We're intending to gradually improve all of the existing functions that
fail these checks as a separate project from other work, because fixing
for these particular lint rules tends to be too invasive to be safe or
sensible to combine with other work.
Therefore we'll temporarily disable these lints from the main lint run
and add a separate .golangci-complexity.yml that we can use to track our
progress towards eliminating those lint failures without continuing to
litter the code with nolint comments in the meantime.
This also removes all of the existing nolint comments for these linters so
that we can start fresh and review each one as part of our improvement
project.
We'll re-enable these linters (and remove .golangci-complexity.yml) once
each example has either been rewritten to pass the checks or we've
concluded that further decomposition would hurt readability and so added
"nolint" comments back in so we can review whether our lint rules are too
strict once we've got a bunch of examples to consider together.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
We currently have a set of aspirational linting rules in the project's
golangci-lint configuration, but this codebase was derived from a much
older codebase that was not written under those lint rules and so we made
the pragmatic decision that only code that has changed since the addition
of the lint rules is subjected to those lint rules.
That approach aims to make the compromise of encouraging us to gradually
improve code "while we're in the area" working on other changes, while
avoiding the need for a huge retrofit of existing code.
However, that compromise seems to be less appropriate for the subset of
linting rules related to code complexity in particular. That category of
rules typically imposes some arbitrary limit on a qualitative metric that
the linting tool can measure. These particular rules therefore have a
relatively broad scope and tend to require very disruptive changes to
existing code in order to resolve them.
This proposal aims to find a pragmatic path that will lead to a codebase
that _does_ conform to the complexity lint rules in the long run, but to
treat those improvements as a separate project in their own right rather
than as something we aim to gradually improve as part of other work.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
By building a map from module path to snapshotModule we can avoid repeatedly scanning the set of modules for each call to Open.
Signed-off-by: Jon Johnson <jon.johnson@chainguard.dev>
Using the word "iteration" to describe what count and for_each do tends to
confuse people because it sounds like explicit control flow rather than
just dynamically declaring multiple objects.
Elsewhere in the codebase we refer to this idea as "expansion" so this is
a rename for consistency with that and to remove the confusing terminology.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
Previously we made a very generic suggestion to use -target to split a
change into two parts as a workaround for the fact that count and for_each
must be known during planning. That works, but we didn't have enough
information available to tell the operator exactly what to target and so
anyone who is not an expert on the configuration they're working with tends
to get stuck unable to figure out exactly what they need to do.
The new -exclude option gives us an opportunity to do better here: we tend
to know for which object we're currently evaluating count or for_each, and
so we can mention that object directly in the error message when if we
recommend to use -exclude instead of -target.
Not all objects that support count/for_each will necessarily be directly
targetable, so we can still potentially recommend -target when we're
dealing with one of those objects. For example, as of this commit that
is true for for_each in a provider block, because there is not currently
any syntax for specifying a provider configuration as an addrs.Targetable.
Perhaps we'll introduce such a thing in the future, but that's outside the
scope of this change that's primarily focused on improving the messaging
for resource and module count/for_each.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
The original proposal called for the state snapshot loader to accept a
resource instance with both an instance-level provider instance address
and a resource-level provider instance address.
The final implementation does follow that specification, but it also emits
a warning in that case to draw attention to the inconsistency.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
The original proposal called for the state snapshot writer to generate a
resource-level provider property if all of the instances of the resource
had the same provider instance address, regardless of what that address
actually is.
The actual implementation instead chose to generate the resource-level
property only if none of the instances of the resource refer to a provider
instance that has an instance key.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
Kind of odd code smell, but the only alternative I could think of was a
panic. Would rather ensure this requirement at compile time instead.
Signed-off-by: Christian Mesh <christianmesh1@gmail.com>
In the main language runtime input variables have both a "raw" value as
provided by the caller and a "finalized" value that has been
type-converted, default-attributes-inserted, and validated.
Unfortunately the "early eval" codepath is essentially a reimplementation
of the language runtime in terms of data available in the static
configuration, and it previously wasn't properly emulating the finalization
of input variable values and was thus incorrectly exposing the "raw"
values into a module instead of the "finalized" values.
Since we are already in the v1.9 prerelease period significant refactoring
is too risky, and so this just copies the most important transformations
from the language runtime into the early eval runtime. We hope to find a
more sustainable way to implement this in the future, but that will likely
require refactoring of both the early eval codepath _and_ the traditional
language runtime, and so that work needs to begin early in a minor release
period.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
