From b2c985cc62ffe4eebbbcad1bcb915c5b70902865 Mon Sep 17 00:00:00 2001 From: rajeshal Date: Mon, 29 Jul 2024 23:45:10 +0530 Subject: [PATCH] review changes Signed-off-by: rajeshal --- clients/go/msd/client.go | 6 +-- clients/go/msd/model.go | 12 +++--- clients/go/msd/msd_schema.go | 38 +++++++++---------- .../athenz/msd/MSDRDLGeneratedClient.java | 9 +++-- .../yahoo/athenz/msd/CompositeInstance.java | 2 + .../java/com/yahoo/athenz/msd/MSDSchema.java | 10 ++--- core/msd/src/main/rdl/Workload.rdli | 6 +-- core/msd/src/main/rdl/Workload.tdl | 4 +- .../athenz/msd/CompositeInstanceTest.java | 38 ------------------- 9 files changed, 44 insertions(+), 81 deletions(-) diff --git a/clients/go/msd/client.go b/clients/go/msd/client.go index b400a8640ba..0ad4410d80b 100644 --- a/clients/go/msd/client.go +++ b/clients/go/msd/client.go @@ -851,7 +851,7 @@ func (client MSDClient) GetWorkloadsByDomainAndService(request *BulkWorkloadRequ } } -func (client MSDClient) PostCompositeInstance(domainName DomainName, serviceName EntityName, instance *CompositeInstance) error { +func (client MSDClient) PutCompositeInstance(domainName DomainName, serviceName EntityName, instance *CompositeInstance) error { url := client.URL + "/domain/" + fmt.Sprint(domainName) + "/service/" + fmt.Sprint(serviceName) + "/workload/discover/instance" contentBytes, err := json.Marshal(instance) if err != nil { @@ -882,8 +882,8 @@ func (client MSDClient) PostCompositeInstance(domainName DomainName, serviceName } } -func (client MSDClient) DeleteCompositeInstance(domainName DomainName, serviceName EntityName, instance *CompositeInstance) error { - url := client.URL + "/domain/" + fmt.Sprint(domainName) + "/service/" + fmt.Sprint(serviceName) + "/workload/discover/instance" +func (client MSDClient) DeleteCompositeInstance(domainName DomainName, serviceName EntityName, instance SimpleName) error { + url := client.URL + "/domain/" + fmt.Sprint(domainName) + "/service/" + fmt.Sprint(serviceName) + "/workload/discover/instance/$" + fmt.Sprint(instance) resp, err := client.httpDelete(url, nil) if err != nil { return err diff --git a/clients/go/msd/model.go b/clients/go/msd/model.go index 7941d67c32e..7159d906fe3 100644 --- a/clients/go/msd/model.go +++ b/clients/go/msd/model.go @@ -2287,12 +2287,12 @@ type CompositeInstance struct { // // instance name/id // - Instance EntityName `json:"instance"` + Instance SimpleName `json:"instance"` // // instance type // - InstanceType string `json:"instanceType"` + InstanceType string `json:"instanceType" rdl:"optional" yaml:",omitempty"` // // name of the instance provider, for example aws/gcp @@ -2356,14 +2356,12 @@ func (self *CompositeInstance) Validate() error { if self.Instance == "" { return fmt.Errorf("CompositeInstance.instance is missing but is a required field") } else { - val := rdl.Validate(MSDSchema(), "EntityName", self.Instance) + val := rdl.Validate(MSDSchema(), "SimpleName", self.Instance) if !val.Valid { - return fmt.Errorf("CompositeInstance.instance does not contain a valid EntityName (%v)", val.Error) + return fmt.Errorf("CompositeInstance.instance does not contain a valid SimpleName (%v)", val.Error) } } - if self.InstanceType == "" { - return fmt.Errorf("CompositeInstance.instanceType is missing but is a required field") - } else { + if self.InstanceType != "" { val := rdl.Validate(MSDSchema(), "String", self.InstanceType) if !val.Valid { return fmt.Errorf("CompositeInstance.instanceType does not contain a valid String (%v)", val.Error) diff --git a/clients/go/msd/msd_schema.go b/clients/go/msd/msd_schema.go index 4eb2436b8bd..cc7062ebc9b 100644 --- a/clients/go/msd/msd_schema.go +++ b/clients/go/msd/msd_schema.go @@ -329,8 +329,8 @@ func init() { tCompositeInstance.Comment("generic instance") tCompositeInstance.Field("domainName", "DomainName", false, nil, "name of the domain") tCompositeInstance.Field("serviceName", "EntityName", false, nil, "name of the service") - tCompositeInstance.Field("instance", "EntityName", false, nil, "instance name/id") - tCompositeInstance.Field("instanceType", "String", false, nil, "instance type") + tCompositeInstance.Field("instance", "SimpleName", false, nil, "instance name/id") + tCompositeInstance.Field("instanceType", "String", true, nil, "instance type") tCompositeInstance.Field("provider", "String", true, nil, "name of the instance provider, for example aws/gcp") tCompositeInstance.Field("certExpiryTime", "Timestamp", true, nil, "certificate expiry time (ex: getNotAfter), if applicable") tCompositeInstance.Field("certIssueTime", "Timestamp", true, nil, "certificate issue time (ex: getNotBefore), if applicable") @@ -896,27 +896,27 @@ func init() { mGetWorkloadsByDomainAndService.Exception("UNAUTHORIZED", "ResourceError", "") sb.AddResource(mGetWorkloadsByDomainAndService.Build()) - mPostCompositeInstance := rdl.NewResourceBuilder("Workloads", "PUT", "/domain/{domainName}/service/{serviceName}/workload/discover/instance") - mPostCompositeInstance.Comment("Api to discover an additional instance which can have static or dynamic or both IPs") - mPostCompositeInstance.Name("postCompositeInstance") - mPostCompositeInstance.Input("domainName", "DomainName", true, "", "", false, nil, "name of the domain") - mPostCompositeInstance.Input("serviceName", "EntityName", true, "", "", false, nil, "name of the service") - mPostCompositeInstance.Input("instance", "CompositeInstance", false, "", "", false, nil, "Generic instance") - mPostCompositeInstance.Auth("update", "{domainName}:service.{serviceName}", false, "") - mPostCompositeInstance.Expected("NO_CONTENT") - mPostCompositeInstance.Exception("BAD_REQUEST", "ResourceError", "") - mPostCompositeInstance.Exception("FORBIDDEN", "ResourceError", "") - mPostCompositeInstance.Exception("NOT_FOUND", "ResourceError", "") - mPostCompositeInstance.Exception("TOO_MANY_REQUESTS", "ResourceError", "") - mPostCompositeInstance.Exception("UNAUTHORIZED", "ResourceError", "") - sb.AddResource(mPostCompositeInstance.Build()) - - mDeleteCompositeInstance := rdl.NewResourceBuilder("Workloads", "DELETE", "/domain/{domainName}/service/{serviceName}/workload/discover/instance") + mPutCompositeInstance := rdl.NewResourceBuilder("Workloads", "PUT", "/domain/{domainName}/service/{serviceName}/workload/discover/instance") + mPutCompositeInstance.Comment("Api to discover an additional instance which can have static or dynamic or both IPs") + mPutCompositeInstance.Name("putCompositeInstance") + mPutCompositeInstance.Input("domainName", "DomainName", true, "", "", false, nil, "name of the domain") + mPutCompositeInstance.Input("serviceName", "EntityName", true, "", "", false, nil, "name of the service") + mPutCompositeInstance.Input("instance", "CompositeInstance", false, "", "", false, nil, "Generic instance") + mPutCompositeInstance.Auth("update", "{domainName}:service.{serviceName}", false, "") + mPutCompositeInstance.Expected("NO_CONTENT") + mPutCompositeInstance.Exception("BAD_REQUEST", "ResourceError", "") + mPutCompositeInstance.Exception("FORBIDDEN", "ResourceError", "") + mPutCompositeInstance.Exception("NOT_FOUND", "ResourceError", "") + mPutCompositeInstance.Exception("TOO_MANY_REQUESTS", "ResourceError", "") + mPutCompositeInstance.Exception("UNAUTHORIZED", "ResourceError", "") + sb.AddResource(mPutCompositeInstance.Build()) + + mDeleteCompositeInstance := rdl.NewResourceBuilder("Workloads", "DELETE", "/domain/{domainName}/service/{serviceName}/workload/discover/instance/${instance}") mDeleteCompositeInstance.Comment("Api to delete an additional instance which can have static or dynamic or both IPs") mDeleteCompositeInstance.Name("deleteCompositeInstance") mDeleteCompositeInstance.Input("domainName", "DomainName", true, "", "", false, nil, "name of the domain") mDeleteCompositeInstance.Input("serviceName", "EntityName", true, "", "", false, nil, "name of the service") - mDeleteCompositeInstance.Input("instance", "CompositeInstance", false, "", "", false, nil, "Generic instance") + mDeleteCompositeInstance.Input("instance", "SimpleName", true, "", "", false, nil, "instance name/id/key") mDeleteCompositeInstance.Auth("update", "{domainName}:service.{serviceName}", false, "") mDeleteCompositeInstance.Expected("NO_CONTENT") mDeleteCompositeInstance.Exception("BAD_REQUEST", "ResourceError", "") diff --git a/clients/java/msd/src/main/java/com/yahoo/athenz/msd/MSDRDLGeneratedClient.java b/clients/java/msd/src/main/java/com/yahoo/athenz/msd/MSDRDLGeneratedClient.java index a6f201eb6d1..2d4664a9006 100644 --- a/clients/java/msd/src/main/java/com/yahoo/athenz/msd/MSDRDLGeneratedClient.java +++ b/clients/java/msd/src/main/java/com/yahoo/athenz/msd/MSDRDLGeneratedClient.java @@ -672,7 +672,7 @@ public BulkWorkloadResponse getWorkloadsByDomainAndService(BulkWorkloadRequest r } } - public Workloads postCompositeInstance(String domainName, String serviceName, CompositeInstance instance) throws URISyntaxException, IOException { + public Workloads putCompositeInstance(String domainName, String serviceName, CompositeInstance instance) throws URISyntaxException, IOException { UriTemplateBuilder uriTemplateBuilder = new UriTemplateBuilder(baseUrl, "/domain/{domainName}/service/{serviceName}/workload/discover/instance") .resolveTemplate("domainName", domainName) .resolveTemplate("serviceName", serviceName); @@ -703,10 +703,11 @@ public Workloads postCompositeInstance(String domainName, String serviceName, Co } } - public Workloads deleteCompositeInstance(String domainName, String serviceName, CompositeInstance instance) throws URISyntaxException, IOException { - UriTemplateBuilder uriTemplateBuilder = new UriTemplateBuilder(baseUrl, "/domain/{domainName}/service/{serviceName}/workload/discover/instance") + public Workloads deleteCompositeInstance(String domainName, String serviceName, String instance) throws URISyntaxException, IOException { + UriTemplateBuilder uriTemplateBuilder = new UriTemplateBuilder(baseUrl, "/domain/{domainName}/service/{serviceName}/workload/discover/instance/${instance}") .resolveTemplate("domainName", domainName) - .resolveTemplate("serviceName", serviceName); + .resolveTemplate("serviceName", serviceName) + .resolveTemplate("instance", instance); URIBuilder uriBuilder = new URIBuilder(uriTemplateBuilder.getUri()); HttpUriRequest httpUriRequest = RequestBuilder.delete() .setUri(uriBuilder.build()) diff --git a/core/msd/src/main/java/com/yahoo/athenz/msd/CompositeInstance.java b/core/msd/src/main/java/com/yahoo/athenz/msd/CompositeInstance.java index 40fb59ba2b5..58d652cc720 100644 --- a/core/msd/src/main/java/com/yahoo/athenz/msd/CompositeInstance.java +++ b/core/msd/src/main/java/com/yahoo/athenz/msd/CompositeInstance.java @@ -15,6 +15,8 @@ public class CompositeInstance { public String domainName; public String serviceName; public String instance; + @RdlOptional + @JsonInclude(JsonInclude.Include.NON_EMPTY) public String instanceType; @RdlOptional @JsonInclude(JsonInclude.Include.NON_EMPTY) diff --git a/core/msd/src/main/java/com/yahoo/athenz/msd/MSDSchema.java b/core/msd/src/main/java/com/yahoo/athenz/msd/MSDSchema.java index 22fe28ab45a..f4cf01cfa9a 100644 --- a/core/msd/src/main/java/com/yahoo/athenz/msd/MSDSchema.java +++ b/core/msd/src/main/java/com/yahoo/athenz/msd/MSDSchema.java @@ -282,8 +282,8 @@ private static Schema build() { .comment("generic instance") .field("domainName", "DomainName", false, "name of the domain") .field("serviceName", "EntityName", false, "name of the service") - .field("instance", "EntityName", false, "instance name/id") - .field("instanceType", "String", false, "instance type") + .field("instance", "SimpleName", false, "instance name/id") + .field("instanceType", "String", true, "instance type") .field("provider", "String", true, "name of the instance provider, for example aws/gcp") .field("certExpiryTime", "Timestamp", true, "certificate expiry time (ex: getNotAfter), if applicable") .field("certIssueTime", "Timestamp", true, "certificate issue time (ex: getNotBefore), if applicable"); @@ -881,7 +881,7 @@ private static Schema build() { sb.resource("CompositeInstance", "PUT", "/domain/{domainName}/service/{serviceName}/workload/discover/instance") .comment("Api to discover an additional instance which can have static or dynamic or both IPs") - .name("postCompositeInstance") + .name("putCompositeInstance") .pathParam("domainName", "DomainName", "name of the domain") .pathParam("serviceName", "EntityName", "name of the service") .input("instance", "CompositeInstance", "Generic instance") @@ -898,12 +898,12 @@ private static Schema build() { .exception("UNAUTHORIZED", "ResourceError", "") ; - sb.resource("Workloads", "DELETE", "/domain/{domainName}/service/{serviceName}/workload/discover/instance") + sb.resource("Workloads", "DELETE", "/domain/{domainName}/service/{serviceName}/workload/discover/instance/${instance}") .comment("Api to delete an additional instance which can have static or dynamic or both IPs") .name("deleteCompositeInstance") .pathParam("domainName", "DomainName", "name of the domain") .pathParam("serviceName", "EntityName", "name of the service") - .input("instance", "CompositeInstance", "Generic instance") + .pathParam("instance", "SimpleName", "instance name/id/key") .auth("update", "{domainName}:service.{serviceName}") .expected("NO_CONTENT") .exception("BAD_REQUEST", "ResourceError", "") diff --git a/core/msd/src/main/rdl/Workload.rdli b/core/msd/src/main/rdl/Workload.rdli index e92d5ffa66d..78df27a4181 100644 --- a/core/msd/src/main/rdl/Workload.rdli +++ b/core/msd/src/main/rdl/Workload.rdli @@ -148,7 +148,7 @@ resource BulkWorkloadResponse POST "/workloads" (name=getWorkloadsByDomainAndSer } // Api to discover an additional instance which can have static or dynamic or both IPs -resource Workloads PUT "/domain/{domainName}/service/{serviceName}/workload/discover/instance" (name=postCompositeInstance) { +resource Workloads PUT "/domain/{domainName}/service/{serviceName}/workload/discover/instance" (name=putCompositeInstance) { DomainName domainName; // name of the domain EntityName serviceName; // name of the service CompositeInstance instance; // Generic instance @@ -164,10 +164,10 @@ resource Workloads PUT "/domain/{domainName}/service/{serviceName}/workload/disc } // Api to delete an additional instance which can have static or dynamic or both IPs -resource Workloads DELETE "/domain/{domainName}/service/{serviceName}/workload/discover/instance" (name=deleteCompositeInstance) { +resource Workloads DELETE "/domain/{domainName}/service/{serviceName}/workload/discover/instance/${instance}" (name=deleteCompositeInstance) { DomainName domainName; // name of the domain EntityName serviceName; // name of the service - CompositeInstance instance; // Generic instance + SimpleName instance; // instance name/id/key authorize ("update", "{domainName}:service.{serviceName}"); expected NO_CONTENT; exceptions { diff --git a/core/msd/src/main/rdl/Workload.tdl b/core/msd/src/main/rdl/Workload.tdl index 5b4dd905d6f..d675eceef85 100644 --- a/core/msd/src/main/rdl/Workload.tdl +++ b/core/msd/src/main/rdl/Workload.tdl @@ -93,8 +93,8 @@ type BulkWorkloadResponse Struct { type CompositeInstance Struct { DomainName domainName; // name of the domain EntityName serviceName; // name of the service - EntityName instance; // instance name/id - String instanceType; // instance type + SimpleName instance; // instance name/id + String instanceType (optional); // instance type String provider (optional); // name of the instance provider, for example aws/gcp Timestamp certExpiryTime (optional); // certificate expiry time (ex: getNotAfter), if applicable Timestamp certIssueTime (optional); // certificate issue time (ex: getNotBefore), if applicable diff --git a/core/msd/src/test/java/com/yahoo/athenz/msd/CompositeInstanceTest.java b/core/msd/src/test/java/com/yahoo/athenz/msd/CompositeInstanceTest.java index eb3eb0d5c70..7dc9e7fa479 100644 --- a/core/msd/src/test/java/com/yahoo/athenz/msd/CompositeInstanceTest.java +++ b/core/msd/src/test/java/com/yahoo/athenz/msd/CompositeInstanceTest.java @@ -114,43 +114,5 @@ public void testCompositeInstanceFields() { assertNotEquals(instance, "mystring"); assertEquals(instance, instance); - } -// -// @Test (dataProvider = "staticWorkloadNameProvider") -// public void testStaticWorkloadName(String name, boolean expected) { -// -// Schema schema = MSDSchema.instance(); -// Validator validator = new Validator(schema); -// -// StaticWorkload wl1 = new StaticWorkload(); -// wl1.setDomainName("athenz") -// .setServiceName("api") -// .setName(name) -// .setType(StaticWorkloadType.CLOUD_LB); -// -// Validator.Result result = validator.validate(wl1, "StaticWorkload"); -// assertEquals(result.valid, expected); -// } -// -// @DataProvider -// private Object[][] staticWorkloadNameProvider() { -// return new Object[][] { -// {"10.10.20.30", true}, -// {"10.10.20.30/24", true}, -// {"172.30.255.255", true}, -// {"2001:db8:abcd:12::ffff", true}, -// {"2001:db8:abcd:12::ffff/24", true}, -// {"2001:db8:abcd:12::ffff/128", true}, -// {"myhostname", true}, -// {"ABC::AA012_113_3332_11344", true}, -// {"myhostname.subdomain.domain.com", true}, -// {"avc/dd", false}, -// {"avc/12/11", false}, -// {"*ddw$%#", false}, -// {"/", false}, -// {"/etc/passwd", false}, -// }; -// } - } \ No newline at end of file