Skip to content

Commit

Permalink
Remove key field from provider.Manifest in the k8s plugin (#5446)
Browse files Browse the repository at this point in the history 
* Dynamically calculate resource keys

Signed-off-by: Shinnosuke Sawada-Dazai <[email protected]>

* Add test for executing K8S_SYNC stage with input namespace in deployment service

Signed-off-by: Shinnosuke Sawada-Dazai <[email protected]>

* Refactor deployment service tests to improve setup and reduce duplication

Signed-off-by: Shinnosuke Sawada-Dazai <[email protected]>

* Remove empty line in imports

Signed-off-by: Shinnosuke Sawada-Dazai <[email protected]>

* Rename Revision field to CommitHash in tests

Signed-off-by: Shinnosuke Sawada-Dazai <[email protected]>

---------

Signed-off-by: Shinnosuke Sawada-Dazai <[email protected]>
  • Loading branch information
@Warashi
Warashi authored Dec 25, 2024
1 parent ef4f6d3 commit 336d96d
Show file tree
Hide file tree
Showing 6 changed files with 126 additions and 75 deletions.
111 changes: 99 additions & 12 deletions pkg/app/pipedv1/plugin/kubernetes/deployment/server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"k8s.io/client-go/tools/clientcmd"
clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
"sigs.k8s.io/controller-runtime/pkg/envtest"
"sigs.k8s.io/yaml"

kubeConfigPkg "github.com/pipe-cd/pipecd/pkg/app/pipedv1/plugin/kubernetes/config"
config "github.com/pipe-cd/pipecd/pkg/configv1"
Expand Down Expand Up @@ -84,19 +85,20 @@ func kubeconfigFromRestConfig(restConfig *rest.Config) (string, error) {
return string(b), nil
}

func TestDeploymentService_executeK8sSyncStage(t *testing.T) {
ctx := context.Background()

// initialize tool registry
testRegistry, err := toolregistrytest.NewToolRegistry(t)
require.NoError(t, err)
func setupEnvTest(t *testing.T) *rest.Config {
t.Helper()

// initialize envtest
tEnv := new(envtest.Environment)
kubeCfg, err := tEnv.Start()
require.NoError(t, err)
t.Cleanup(func() { tEnv.Stop() })

return kubeCfg
}

func setupTestPluginConfig(t *testing.T, kubeCfg *rest.Config) *config.PipedPlugin {
t.Helper()

kubeconfig, err := kubeconfigFromRestConfig(kubeCfg)
require.NoError(t, err)

Expand All @@ -109,7 +111,7 @@ func TestDeploymentService_executeK8sSyncStage(t *testing.T) {
require.NoError(t, err)

// prepare the piped plugin config
pluginCfg := &config.PipedPlugin{
return &config.PipedPlugin{
Name: "kubernetes",
URL: "file:///path/to/kubernetes/plugin", // dummy for testing
Port: 0, // dummy for testing
Expand All @@ -119,10 +121,28 @@ func TestDeploymentService_executeK8sSyncStage(t *testing.T) {
Config: json.RawMessage(deployTarget),
}},
}
}

func setupTestPluginConfigAndDynamicClient(t *testing.T) (*config.PipedPlugin, dynamic.Interface) {
t.Helper()

kubeCfg := setupEnvTest(t)
pluginCfg := setupTestPluginConfig(t, kubeCfg)

dynamicClient, err := dynamic.NewForConfig(kubeCfg)
require.NoError(t, err)

return pluginCfg, dynamicClient
}

func TestDeploymentService_executeK8sSyncStage(t *testing.T) {
ctx := context.Background()

// read the application config from the example file
cfg, err := os.ReadFile(filepath.Join(examplesDir(), "kubernetes", "simple", "app.pipecd.yaml"))
require.NoError(t, err)

// prepare the request
req := &deployment.ExecuteStageRequest{
Input: &deployment.ExecutePluginInput{
Deployment: &model.Deployment{
Expand All @@ -145,16 +165,19 @@ func TestDeploymentService_executeK8sSyncStage(t *testing.T) {
},
}

// initialize tool registry
testRegistry, err := toolregistrytest.NewToolRegistry(t)
require.NoError(t, err)

// initialize plugin config and dynamic client for assertions with envtest
pluginCfg, dynamicClient := setupTestPluginConfigAndDynamicClient(t)

svc := NewDeploymentService(pluginCfg, zaptest.NewLogger(t), testRegistry, logpersistertest.NewTestLogPersister(t))
resp, err := svc.ExecuteStage(ctx, req)

require.NoError(t, err)
assert.Equal(t, model.StageStatus_STAGE_SUCCESS.String(), resp.GetStatus().String())

// check the deployment is created with client-go
dynamicClient, err := dynamic.NewForConfig(kubeCfg)
require.NoError(t, err)

deployment, err := dynamicClient.Resource(schema.GroupVersionResource{Group: "apps", Version: "v1", Resource: "deployments"}).Namespace("default").Get(context.Background(), "simple", metav1.GetOptions{})
require.NoError(t, err)

Expand All @@ -168,3 +191,67 @@ func TestDeploymentService_executeK8sSyncStage(t *testing.T) {
assert.Equal(t, "0123456789", deployment.GetAnnotations()["pipecd.dev/commit-hash"])

}

func TestDeploymentService_executeK8sSyncStage_withInputNamespace(t *testing.T) {
ctx := context.Background()

// read the application config from the example file
cfg, err := os.ReadFile(filepath.Join(examplesDir(), "kubernetes", "simple", "app.pipecd.yaml"))
require.NoError(t, err)

// decode and override the autoCreateNamespace and namespace
spec, err := config.DecodeYAML[*kubeConfigPkg.KubernetesApplicationSpec](cfg)
require.NoError(t, err)
spec.Spec.Input.AutoCreateNamespace = true
spec.Spec.Input.Namespace = "test-namespace"
cfg, err = yaml.Marshal(spec)
require.NoError(t, err)

// prepare the request
req := &deployment.ExecuteStageRequest{
Input: &deployment.ExecutePluginInput{
Deployment: &model.Deployment{
PipedId: "piped-id",
ApplicationId: "app-id",
DeployTargets: []string{"default"},
},
Stage: &model.PipelineStage{
Id: "stage-id",
Name: "K8S_SYNC",
},
StageConfig: []byte(``),
RunningDeploymentSource: nil,
TargetDeploymentSource: &deployment.DeploymentSource{
ApplicationDirectory: filepath.Join(examplesDir(), "kubernetes", "simple"),
CommitHash: "0123456789",
ApplicationConfig: cfg,
ApplicationConfigFilename: "app.pipecd.yaml",
},
},
}

// initialize tool registry
testRegistry, err := toolregistrytest.NewToolRegistry(t)
require.NoError(t, err)

// initialize plugin config and dynamic client for assertions with envtest
pluginCfg, dynamicClient := setupTestPluginConfigAndDynamicClient(t)

svc := NewDeploymentService(pluginCfg, zaptest.NewLogger(t), testRegistry, logpersistertest.NewTestLogPersister(t))
resp, err := svc.ExecuteStage(ctx, req)

require.NoError(t, err)
assert.Equal(t, model.StageStatus_STAGE_SUCCESS.String(), resp.GetStatus().String())

deployment, err := dynamicClient.Resource(schema.GroupVersionResource{Group: "apps", Version: "v1", Resource: "deployments"}).Namespace("test-namespace").Get(context.Background(), "simple", metav1.GetOptions{})
require.NoError(t, err)

assert.Equal(t, "simple", deployment.GetName())
assert.Equal(t, "simple", deployment.GetLabels()["app"])
assert.Equal(t, "piped", deployment.GetAnnotations()["pipecd.dev/managed-by"])
assert.Equal(t, "piped-id", deployment.GetAnnotations()["pipecd.dev/piped"])
assert.Equal(t, "app-id", deployment.GetAnnotations()["pipecd.dev/application"])
assert.Equal(t, "apps/v1", deployment.GetAnnotations()["pipecd.dev/original-api-version"])
assert.Equal(t, "apps/v1:Deployment::simple", deployment.GetAnnotations()["pipecd.dev/resource-key"]) // This assertion differs from the non-plugin-arched piped's Kubernetes platform provider, but we decided to change this behavior.
assert.Equal(t, "0123456789", deployment.GetAnnotations()["pipecd.dev/commit-hash"])
}
35 changes: 25 additions & 10 deletions pkg/app/pipedv1/plugin/kubernetes/provider/applier_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"testing"

"go.uber.org/zap"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"

"github.com/pipe-cd/pipecd/pkg/app/pipedv1/plugin/kubernetes/config"
)
Expand Down Expand Up @@ -164,8 +165,12 @@ func TestApplier_ApplyManifest(t *testing.T) {
)

manifest := Manifest{
key: ResourceKey{
namespace: "test-namespace",
body: &unstructured.Unstructured{
Object: map[string]interface{}{
"metadata": map[string]interface{}{
"namespace": "test-namespace",
},
},
},
}

Expand Down Expand Up @@ -254,8 +259,12 @@ func TestApplier_CreateManifest(t *testing.T) {
)

manifest := Manifest{
key: ResourceKey{
namespace: "test-namespace",
body: &unstructured.Unstructured{
Object: map[string]interface{}{
"metadata": map[string]interface{}{
"namespace": "test-namespace",
},
},
},
}

Expand Down Expand Up @@ -316,11 +325,14 @@ func TestApplier_ReplaceManifest(t *testing.T) {
)

manifest := Manifest{
key: ResourceKey{
namespace: "test-namespace",
body: &unstructured.Unstructured{
Object: map[string]interface{}{
"metadata": map[string]interface{}{
"namespace": "test-namespace",
},
},
},
}

err := applier.ReplaceManifest(context.Background(), manifest)
if !errors.Is(err, tc.expectedErr) {
t.Errorf("expected error %v, got %v", tc.expectedErr, err)
Expand Down Expand Up @@ -378,11 +390,14 @@ func TestApplier_ForceReplaceManifest(t *testing.T) {
)

manifest := Manifest{
key: ResourceKey{
namespace: "test-namespace",
body: &unstructured.Unstructured{
Object: map[string]interface{}{
"metadata": map[string]interface{}{
"namespace": "test-namespace",
},
},
},
}

err := applier.ForceReplaceManifest(context.Background(), manifest)
if !errors.Is(err, tc.expectedErr) {
t.Errorf("expected error %v, got %v", tc.expectedErr, err)
Expand Down
15 changes: 0 additions & 15 deletions pkg/app/pipedv1/plugin/kubernetes/provider/loader.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,6 @@ func NewLoader(registry ToolRegistry) *Loader {

func (l *Loader) LoadManifests(ctx context.Context, input LoaderInput) (manifests []Manifest, err error) {
defer func() {
// Override namespace if set because ParseManifests does not parse it
// if namespace is not explicitly specified in the manifests.
setNamespace(manifests, input.Namespace)

// Add builtin annotations for tracking application live state.
for i := range manifests {
manifests[i].AddAnnotations(map[string]string{
Expand Down Expand Up @@ -132,16 +128,6 @@ func (l *Loader) LoadManifests(ctx context.Context, input LoaderInput) (manifest
}
}

func setNamespace(manifests []Manifest, namespace string) {
if namespace == "" {
return
}
for i := range manifests {
// TODO: consider way to not modify the original object.
manifests[i].key.namespace = namespace
}
}

func sortManifests(manifests []Manifest) {
if len(manifests) < 2 {
return
Expand Down Expand Up @@ -258,7 +244,6 @@ func ParseManifests(data string) ([]Manifest, error) {
continue
}
manifests = append(manifests, Manifest{
key: MakeResourceKey(&obj),
body: &obj,
})
}
Expand Down
35 changes: 0 additions & 35 deletions pkg/app/pipedv1/plugin/kubernetes/provider/loader_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,6 @@ metadata:
`,
want: []Manifest{
{
key: ResourceKey{
apiVersion: "v1",
kind: "ConfigMap",
name: "test-config",
},
body: &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "v1",
Expand Down Expand Up @@ -90,11 +85,6 @@ metadata:
`,
want: []Manifest{
{
key: ResourceKey{
apiVersion: "v1",
kind: "ConfigMap",
name: "test-config",
},
body: &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "v1",
Expand All @@ -106,11 +96,6 @@ metadata:
},
},
{
key: ResourceKey{
apiVersion: "v1",
kind: "Service",
name: "test-service",
},
body: &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "v1",
Expand Down Expand Up @@ -188,11 +173,6 @@ metadata:
},
want: []Manifest{
{
key: ResourceKey{
apiVersion: "v1",
kind: "ConfigMap",
name: "test-config",
},
body: &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "v1",
Expand Down Expand Up @@ -230,11 +210,6 @@ metadata:
},
want: []Manifest{
{
key: ResourceKey{
apiVersion: "v1",
kind: "Service",
name: "test-service",
},
body: &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "v1",
Expand Down Expand Up @@ -271,11 +246,6 @@ metadata:
},
want: []Manifest{
{
key: ResourceKey{
apiVersion: "v1",
kind: "ConfigMap",
name: "test-config",
},
body: &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "v1",
Expand All @@ -287,11 +257,6 @@ metadata:
},
},
{
key: ResourceKey{
apiVersion: "v1",
kind: "Service",
name: "test-service",
},
body: &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "v1",
Expand Down
3 changes: 1 addition & 2 deletions pkg/app/pipedv1/plugin/kubernetes/provider/manifest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,11 @@ import (

// Manifest represents a Kubernetes resource manifest.
type Manifest struct {
key ResourceKey
body *unstructured.Unstructured
}

func (m *Manifest) Key() ResourceKey {
return m.key
return makeResourceKey(m.body)
}

// UnmarshalJSON implements the json.Unmarshaler interface.
Expand Down
2 changes: 1 addition & 1 deletion pkg/app/pipedv1/plugin/kubernetes/provider/resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func (k ResourceKey) ReadableString() string {
return fmt.Sprintf("name=%q, kind=%q, namespace=%q, apiVersion=%q", k.name, k.kind, k.namespace, k.apiVersion)
}

func MakeResourceKey(obj *unstructured.Unstructured) ResourceKey {
func makeResourceKey(obj *unstructured.Unstructured) ResourceKey {
k := ResourceKey{
apiVersion: obj.GetAPIVersion(),
kind: obj.GetKind(),
Expand Down

0 comments on commit 336d96d

Please sign in to comment.