Skip to content
This repository has been archived by the owner on Nov 30, 2023. It is now read-only.

Add warning message for deprecated "hashicorp/terraform" provider #111

Merged
merged 10 commits into from
Oct 11, 2023
48 changes: 48 additions & 0 deletions src/internal/warnings/warnings.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Package warnings defines the warnings associated with the provider
package warnings

import "context"

// TODO: add warnings for any archived providers based on the github repo status
kislerdm marked this conversation as resolved.
Show resolved Hide resolved
// TODO: consider more scalable approach to warn users, do we need it at all?
// TODO: How to govern the warnings, i.e. how to align their correctness with provider maintainers?

// ProviderWarnings return the list of warnings for a given provider identified by its namespace and type
//
// Example: registry.terraform.io/hashicorp/terraform
//
// warn := ProviderWarnings("hashicorp", "terraform")
// fmt.Println(warn)
// >> [This provider is archived and no longer needed. The terraform_remote_state data source is built into the latest OpenTofu release.]
func ProviderWarnings(providerNamespace, providerType string) []string {
switch providerNamespace { //nolint:gocritic // Switch is more appropriate than 'if' for the use case
case "hashicorp":
switch providerType { //nolint:gocritic // Switch is more appropriate than 'if' for the use case
case "terraform":
return []string{`This provider is archived and no longer needed. The terraform_remote_state data source is built into the latest OpenTofu release.`}
}
}

return nil
}

var warningsContext = struct{}{} //nolint:gochecknoglobals // This is a commonly used pattern for context binding
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey! Thanks for suggestion, it does make sense indeed! I've implemented the change accordingly, however the key is called contextKey because it's obvious that it relates to warnings as it's internal to the warnings package. Please review the change and resolve the comment. Thanks!


// NewContext adds warnings to the parent context.
func NewContext(ctx context.Context, warnings []string) context.Context {
if ctx == nil {
panic("ctx must be provided")
}
if len(warnings) == 0 {
return ctx
}
return context.WithValue(ctx, warningsContext, warnings)
}

// FromContext extracts the list of warnings from the context.
func FromContext(ctx context.Context) []string {
if v := ctx.Value(warningsContext); v != nil {
return v.([]string)
}
return nil
}
151 changes: 151 additions & 0 deletions src/internal/warnings/warnings_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
package warnings

import (
"context"
"reflect"
"testing"
)

func TestProviderWarnings(t *testing.T) {
type args struct {
providerNamespace string
providerType string
}
tests := []struct {
name string
args args
want []string
}{
{
name: "shall return no warnings",
args: args{
providerNamespace: "foo",
providerType: "bar",
},
want: nil,
},
{
name: "shall return warnings as in https://github.com/opentofu/registry/issues/108",
args: args{
providerNamespace: "hashicorp",
providerType: "terraform",
},
want: []string{`This provider is archived and no longer needed. The terraform_remote_state data source is built into the latest OpenTofu release.`},
},
}
for _, tt := range tests {
t.Run(
tt.name, func(t *testing.T) {
if got := ProviderWarnings(tt.args.providerNamespace, tt.args.providerType); !reflect.DeepEqual(got, tt.want) {
t.Errorf("ProviderWarnings() = %v, want %v", got, tt.want)
}
},
)
}
}

func TestNewContext(t *testing.T) {
type args struct {
ctx context.Context
warnings []string
}
tests := []struct {
name string
args args
wantIdenticalToInput bool
isPanic bool
}{
{
name: "shall panic on nil parent context",
args: args{
ctx: nil,
warnings: nil,
},
isPanic: true,
},
{
name: "shall return non identical context",
args: args{
ctx: context.TODO(),
warnings: []string{"foo"},
},
isPanic: false,
wantIdenticalToInput: false,
},
{
name: "shall return identical context - nil warnings",
args: args{
ctx: context.TODO(),
warnings: nil,
},
isPanic: false,
wantIdenticalToInput: true,
},
{
name: "shall return identical context - zero-length array of warnings",
args: args{
ctx: context.TODO(),
warnings: []string{},
},
isPanic: false,
wantIdenticalToInput: true,
},
}
for _, tt := range tests {
t.Run(
tt.name, func(t *testing.T) {
defer func() {
if r := recover(); (r == nil) == tt.isPanic {
t.Fatal("panic expected")
}
}()
if got := NewContext(tt.args.ctx, tt.args.warnings); reflect.DeepEqual(got, tt.args.ctx) != tt.wantIdenticalToInput {
t.Errorf("expected to be identical to input: %v", tt.wantIdenticalToInput)
}
},
)
}
}

func TestFromContext(t *testing.T) {
type args struct {
ctx context.Context
}

tests := []struct {
name string
args args
want []string
}{
{
name: "shall return nil for ctx w/o the warnings value",
args: args{
ctx: context.TODO(),
},
want: nil,
},
{
name: "shall return nil for ctx w nil warnings",
args: args{
ctx: NewContext(context.TODO(), nil),
},
want: nil,
},
{
name: "shall return warnings for ctx w warnings",
args: args{
ctx: NewContext(context.TODO(), []string{"foo"}),
},
want: []string{"foo"},
},
}
for _, tt := range tests {
t.Run(
tt.name, func(t *testing.T) {
if got := FromContext(tt.args.ctx); !reflect.DeepEqual(got, tt.want) {
t.Errorf("FromContext() = %v, want %v", got, tt.want)
}
},
)
}
}
17 changes: 13 additions & 4 deletions src/lambda/api/providerVersions.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/opentofu/registry/internal/github"
"github.com/opentofu/registry/internal/providers"
"github.com/opentofu/registry/internal/providers/providercache"
"github.com/opentofu/registry/internal/warnings"
"golang.org/x/exp/slog"
)

Expand All @@ -40,6 +41,7 @@ func getListProvidersPathParams(req events.APIGatewayProxyRequest) ListProviders

type ListProviderVersionsResponse struct {
Versions []providers.Version `json:"versions"`
Warnings []string `json:"warnings,omitempty"`
}

func listProviderVersions(config config.Config) LambdaFunc {
Expand All @@ -50,6 +52,9 @@ func listProviderVersions(config config.Config) LambdaFunc {
effectiveNamespace := config.EffectiveProviderNamespace(params.Namespace)
repoName := providers.GetRepoName(params.Type)

// Warnings lookup: https://github.com/opentofu/registry/issues/108
ctx = warnings.NewContext(ctx, warnings.ProviderWarnings(params.Namespace, params.Type))

// For now, we will ignore errors from the cache and just fetch from GH instead
document, _ := config.ProviderVersionCache.GetItem(ctx, fmt.Sprintf("%s/%s", effectiveNamespace, params.Type))
if document != nil {
Expand Down Expand Up @@ -82,15 +87,15 @@ func processDocument(ctx context.Context, document *providercache.VersionListing

if time.Since(document.LastUpdated) < providercache.AllowedAge {
slog.Info("Document is not too old, returning cached versions", "last_updated", document.LastUpdated)
return versionsResponse(document.Versions)
return versionsResponse(document.Versions, warnings.FromContext(ctx))
}

slog.Info("Document is too old, triggering lambda to update dynamodb", "last_updated", document.LastUpdated)
if err := triggerPopulateProviderVersions(ctx, config, namespace, providerType); err != nil {
slog.Error("Error triggering lambda", "error", err)
}

return versionsResponse(document.Versions)
return versionsResponse(document.Versions, warnings.FromContext(ctx))
}

func fetchFromGithub(ctx context.Context, config config.Config, namespace, repoName string) (events.APIGatewayProxyResponse, error) {
Expand All @@ -102,7 +107,7 @@ func fetchFromGithub(ctx context.Context, config config.Config, namespace, repoN
return events.APIGatewayProxyResponse{StatusCode: http.StatusInternalServerError}, err
}

return versionsResponse(versions)
return versionsResponse(versions, warnings.FromContext(ctx))
}

func triggerPopulateProviderVersions(ctx context.Context, config config.Config, effectiveNamespace string, effectiveType string) error {
Expand All @@ -120,11 +125,15 @@ func triggerPopulateProviderVersions(ctx context.Context, config config.Config,
return nil
}

func versionsResponse(versions []providers.Version) (events.APIGatewayProxyResponse, error) {
func versionsResponse(versions []providers.Version, warnings []string) (events.APIGatewayProxyResponse, error) {
response := ListProviderVersionsResponse{
Versions: versions,
}

if len(warnings) > 0 {
response.Warnings = warnings
}

resBody, err := json.Marshal(response)
if err != nil {
return events.APIGatewayProxyResponse{StatusCode: http.StatusInternalServerError}, err
Expand Down
Loading