elastic / terraform-provider-elasticstack

Terraform provider for Elastic Stack
https://registry.terraform.io/providers/elastic/elasticstack/latest/docs
Apache License 2.0
155 stars 78 forks source link

Handle Deprecations: Kibana Saved Objects APIs #637

Open TinaHeiligers opened 1 month ago

TinaHeiligers commented 1 month ago

Most of the Kibana Saved Objects APIs were deprecated in 8.7 (released Mar 30, 2023).

The APIs that are to remain are:

/api/saved_objects/_import /api/saved_objects/_export /api/saved_objects/_resolve_import_errors

The provider will need to [change](const ( basePathKibanaSavedObject = "/api/saved_objects" // Base URL to access on Kibana save objects )) the implementation before these APIs are removed.

TinaHeiligers commented 1 month ago

cc @jloleysens

tobio commented 1 month ago

IIUC the provider is only using the _import endpoint now (see here).

Is there something else here that I'm not aware of?

TinaHeiligers commented 1 month ago

IUC the provider is only using the _import endpoint now (see here).

Fantastic!

Is there something else here that I'm not aware of?

No, the _import API isn't deprecated and will be supported for the foreseeable future.

@tobio please let the Kibana Core team know if your team objects to the non-public saved object's APIs being deprecated. Closing this issue as resolved with no action needed.

TinaHeiligers commented 2 weeks ago

Reopened to discuss the follow-up action to delete the deprecated saved objects APIs.

If we plan to support terraform-provider-elasticstack, keeping deprecated APIs around runs the risk of consumers building integrations with APIs that aren't supported anymore, if they even still exist!

We strongly suggest handling the necessary changes a.s.a.p.

Current:

type OptionalFindParameters struct {
    ObjectsPerPage        int
    Page                  int
    Search                string
    DefaultSearchOperator string
    SearchFields          []string
    Fields                []string
    SortField             string
    HasReference          string
}

// KibanaSavedObjectGet permit to get saved object from Kibana
type KibanaSavedObjectGet func(objectType string, id string, kibanaSpace string) (map[string]interface{}, error)

// KibanaSavedObjectFind permit to find saved objects from Kibana
type KibanaSavedObjectFind func(objectType string, kibanaSpace string, optionalParameters *OptionalFindParameters) (map[string]interface{}, error)

// KibanaSavedObjectCreate permit to create saved object in Kibana
type KibanaSavedObjectCreate func(data map[string]interface{}, objectType string, id string, overwrite bool, kibanaSpace string) (map[string]interface{}, error)

// KibanaSavedObjectUpdate permit to update saved object in Kibana
type KibanaSavedObjectUpdate func(data map[string]interface{}, objectType string, id string, kibanaSpace string) (map[string]interface{}, error)

// KibanaSavedObjectDelete permit to delete saved object in Kibana
type KibanaSavedObjectDelete func(objectType string, id string, kibanaSpace string) error

// KibanaSavedObjectExport permit to export saved objects from Kibana
type KibanaSavedObjectExport func(objectTypes []string, objects []map[string]string, deepReference bool, kibanaSpace string) ([]byte, error)

// KibanaSavedObjectImport permit to import saved objects in Kibana
type KibanaSavedObjectImport func(data []byte, overwrite bool, kibanaSpace string) (map[string]interface{}, error)

// String permit to return OptionalFindParameters object as JSON string
func (o *OptionalFindParameters) String() string {
    json, _ := json.Marshal(o)
    return string(json)
}
...rest

Needs to change to:

// KibanaSavedObjectExport permit to export saved objects from Kibana
type KibanaSavedObjectExport func(objectTypes []string, objects []map[string]string, deepReference bool, kibanaSpace string) ([]byte, error)

// KibanaSavedObjectImport permit to import saved objects in Kibana
type KibanaSavedObjectImport func(data []byte, overwrite bool, kibanaSpace string) (map[string]interface{}, error)
...whatever remains
dimuon commented 6 days ago

@TinaHeiligers , thank you for the details.

Does it make sense to delete the deprecated API from the provider? What about old Kibana versions, e.g. if a consumer upgrades the provider to a version without the API, they won't be able to use it against Kibana 8.6. Maybe it makes sense to output a deprecation warning instead if the API is called?

TinaHeiligers commented 6 days ago

@dimuon A deprecation warning for versions < 9 will be needed. If the terraform provider is to support kibana v9, calling these APIs will throw because they won't exist.

dimuon commented 5 days ago

@Kushmaro , WDYT? E.g. we can just show a deprecation warning if the API is called and let it fail if it's used against Kibana v9.

tobio commented 5 days ago

@dimuon in case it wasn't clear, the provider doesn't use any of the deprecated API's. It's just that the code exists in the local fork of the Kibana rest client. I imagine it would be just as much work to add the deprecation log (where? to stdout, that would be pretty annoying for someone importing this code) as it would be to remove the code.

IMO this issue is a pretty low priority compared to actual customer requests in this repo:

dimuon commented 5 days ago

@tobio , thanks for the clarification. Yeah, it makes sense to remove the API.