The attached patch for #3125 [0]is based on Jakub's "secrets" branch on github [1], as there are (at least) a few issues with the current secrets code.
[0]: https://fedorahosted.org/sssd/ticket/312 [1]: https://github.com/jhrozek/sssd/tree/secrets
Although I didn't fire a CI build, the local "make intgcheck" passes without issues.
Best Regards, -- Fabiano Fidêncio
On Wed, Aug 17, 2016 at 03:13:57PM +0200, Fabiano Fidêncio wrote:
The attached patch for #3125 [0]is based on Jakub's "secrets" branch on github [1], as there are (at least) a few issues with the current secrets code.
Although I didn't fire a CI build, the local "make intgcheck" passes without issues.
Best Regards,
Fabiano Fidêncio
Sorry about the late reply. The patch works fine as I would expect:
sudo curl -H "Content-Type: application/json" --unix-socket /var/run/secrets.socket -XDELETE http://localhost/secrets/xxx <html> <head> <title>404 Not Found</title></head> <body> <h1>Not Found</h1> <p>The requested resource was not found.</p> </body>
Nonetheless, I think the patch can be a bit simpler:
From bc39f7ba0ffa75a9ca972266bbc2741cbb0c8c73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= fidencio@redhat.com Date: Wed, 17 Aug 2016 13:12:21 +0200 Subject: [PATCH] SECRETS: Check whether a secret exists before trying to delete it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
Checking whether a secret exists or not before trying to delete it allows SSSD to report the proper error code in case of trying to delete a non-existent secret.
Resolves: https://fedorahosted.org/sssd/ticket/3125
Signed-off-by: Fabiano Fidêncio fidencio@redhat.com
src/responder/secrets/local.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/src/responder/secrets/local.c b/src/responder/secrets/local.c index e91766f..3985b6d 100644 --- a/src/responder/secrets/local.c +++ b/src/responder/secrets/local.c @@ -376,12 +376,26 @@ int local_db_delete(TALLOC_CTX *mem_ctx, struct local_context *lctx, const char *req_path) {
static const char *attrs[] = { "secret", NULL }; struct ldb_dn *dn;
struct ldb_result *res; int ret;
ret = local_db_dn(mem_ctx, lctx->ldb, req_path, &dn); if (ret != EOK) goto done;
ret = ldb_search(lctx->ldb, mem_ctx, &res, dn, LDB_SCOPE_BASE,
attrs, "%s", LOCAL_SIMPLE_FILTER);
if (ret != EOK) {
ret = ENOENT;
goto done;
}
if (res->count == 0) {
ret = ENOENT;
goto done;
}
ret = ldb_delete(lctx->ldb, dn);
I think it's OK to just try to delete the secret and act on the return value. This is what we already have in sysdb_delete_cache_entry(): 105 ret = ldb_delete(ldb, dn); 106 switch (ret) { 107 case LDB_SUCCESS: 108 return EOK; 109 case LDB_ERR_NO_SUCH_OBJECT: 110 if (ignore_not_found) { 111 return EOK; 112 }
So I think we can use something else in secrets to return ENOENT on receiving LDB_ERR_NO_SUCH_OBJECT from ldb_delete().
Jakub,
On Thu, Aug 25, 2016 at 1:56 PM, Jakub Hrozek jhrozek@redhat.com wrote:
On Wed, Aug 17, 2016 at 03:13:57PM +0200, Fabiano Fidêncio wrote:
The attached patch for #3125 [0]is based on Jakub's "secrets" branch on github [1], as there are (at least) a few issues with the current secrets code.
Although I didn't fire a CI build, the local "make intgcheck" passes without issues.
Best Regards,
Fabiano Fidêncio
Sorry about the late reply. The patch works fine as I would expect:
sudo curl -H "Content-Type: application/json" --unix-socket /var/run/secrets.socket -XDELETE http://localhost/secrets/xxx
<html> <head> <title>404 Not Found</title></head> <body> <h1>Not Found</h1> <p>The requested resource was not found.</p> </body>
Nonetheless, I think the patch can be a bit simpler:
From bc39f7ba0ffa75a9ca972266bbc2741cbb0c8c73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= fidencio@redhat.com Date: Wed, 17 Aug 2016 13:12:21 +0200 Subject: [PATCH] SECRETS: Check whether a secret exists before trying to delete it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
Checking whether a secret exists or not before trying to delete it allows SSSD to report the proper error code in case of trying to delete a non-existent secret.
Resolves: https://fedorahosted.org/sssd/ticket/3125
Signed-off-by: Fabiano Fidêncio fidencio@redhat.com
src/responder/secrets/local.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/src/responder/secrets/local.c b/src/responder/secrets/local.c index e91766f..3985b6d 100644 --- a/src/responder/secrets/local.c +++ b/src/responder/secrets/local.c @@ -376,12 +376,26 @@ int local_db_delete(TALLOC_CTX *mem_ctx, struct local_context *lctx, const char *req_path) {
static const char *attrs[] = { "secret", NULL }; struct ldb_dn *dn;
struct ldb_result *res; int ret;
ret = local_db_dn(mem_ctx, lctx->ldb, req_path, &dn); if (ret != EOK) goto done;
ret = ldb_search(lctx->ldb, mem_ctx, &res, dn, LDB_SCOPE_BASE,
attrs, "%s", LOCAL_SIMPLE_FILTER);
if (ret != EOK) {
ret = ENOENT;
goto done;
}
if (res->count == 0) {
ret = ENOENT;
goto done;
}
ret = ldb_delete(lctx->ldb, dn);
I think it's OK to just try to delete the secret and act on the return value. This is what we already have in sysdb_delete_cache_entry(): 105 ret = ldb_delete(ldb, dn); 106 switch (ret) { 107 case LDB_SUCCESS: 108 return EOK; 109 case LDB_ERR_NO_SUCH_OBJECT: 110 if (ignore_not_found) { 111 return EOK; 112 }
So I think we can use something else in secrets to return ENOENT on receiving LDB_ERR_NO_SUCH_OBJECT from ldb_delete().
Suggestion taken. You can find the v2 attached to this email.
Best Regards, -- Fabiano Fidêncio
On (26/08/16 08:10), Fabiano Fidêncio wrote:
Jakub,
On Thu, Aug 25, 2016 at 1:56 PM, Jakub Hrozek jhrozek@redhat.com wrote:
On Wed, Aug 17, 2016 at 03:13:57PM +0200, Fabiano Fidêncio wrote:
The attached patch for #3125 [0]is based on Jakub's "secrets" branch on github [1], as there are (at least) a few issues with the current secrets code.
Although I didn't fire a CI build, the local "make intgcheck" passes without issues.
Best Regards,
Fabiano Fidêncio
Sorry about the late reply. The patch works fine as I would expect:
sudo curl -H "Content-Type: application/json" --unix-socket /var/run/secrets.socket -XDELETE http://localhost/secrets/xxx
<html> <head> <title>404 Not Found</title></head> <body> <h1>Not Found</h1> <p>The requested resource was not found.</p> </body>
Nonetheless, I think the patch can be a bit simpler:
From bc39f7ba0ffa75a9ca972266bbc2741cbb0c8c73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= fidencio@redhat.com Date: Wed, 17 Aug 2016 13:12:21 +0200 Subject: [PATCH] SECRETS: Check whether a secret exists before trying to delete it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
Checking whether a secret exists or not before trying to delete it allows SSSD to report the proper error code in case of trying to delete a non-existent secret.
Resolves: https://fedorahosted.org/sssd/ticket/3125
Signed-off-by: Fabiano Fidêncio fidencio@redhat.com
src/responder/secrets/local.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/src/responder/secrets/local.c b/src/responder/secrets/local.c index e91766f..3985b6d 100644 --- a/src/responder/secrets/local.c +++ b/src/responder/secrets/local.c @@ -376,12 +376,26 @@ int local_db_delete(TALLOC_CTX *mem_ctx, struct local_context *lctx, const char *req_path) {
static const char *attrs[] = { "secret", NULL }; struct ldb_dn *dn;
struct ldb_result *res; int ret;
ret = local_db_dn(mem_ctx, lctx->ldb, req_path, &dn); if (ret != EOK) goto done;
ret = ldb_search(lctx->ldb, mem_ctx, &res, dn, LDB_SCOPE_BASE,
attrs, "%s", LOCAL_SIMPLE_FILTER);
if (ret != EOK) {
ret = ENOENT;
goto done;
}
if (res->count == 0) {
ret = ENOENT;
goto done;
}
ret = ldb_delete(lctx->ldb, dn);
I think it's OK to just try to delete the secret and act on the return value. This is what we already have in sysdb_delete_cache_entry(): 105 ret = ldb_delete(ldb, dn); 106 switch (ret) { 107 case LDB_SUCCESS: 108 return EOK; 109 case LDB_ERR_NO_SUCH_OBJECT: 110 if (ignore_not_found) { 111 return EOK; 112 }
So I think we can use something else in secrets to return ENOENT on receiving LDB_ERR_NO_SUCH_OBJECT from ldb_delete().
Suggestion taken. You can find the v2 attached to this email.
Best Regards,
Fabiano Fidêncio
From 3e71f901ab12f5e5c570dcf0796ef47e379e7a53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= fidencio@redhat.com Date: Wed, 17 Aug 2016 13:12:21 +0200 Subject: [PATCH v2] SECRETS: Return ENOENT when_deleting a non-existent secret MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
The approach taken is quite similar to what is done by sysdb_delete_cache_entry().
Resolves: https://fedorahosted.org/sssd/ticket/3125
Signed-off-by: Fabiano Fidêncio fidencio@redhat.com
src/responder/secrets/local.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/responder/secrets/local.c b/src/responder/secrets/local.c index e91766f..c32af8c 100644 --- a/src/responder/secrets/local.c +++ b/src/responder/secrets/local.c @@ -380,15 +380,17 @@ int local_db_delete(TALLOC_CTX *mem_ctx, int ret;
ret = local_db_dn(mem_ctx, lctx->ldb, req_path, &dn);
- if (ret != EOK) goto done;
if (ret != EOK) return ret;
ret = ldb_delete(lctx->ldb, dn);
- if (ret != EOK) {
ret = EIO;
- switch (ret) {
- case LDB_SUCCESS:
return EOK;
- case LDB_ERR_NO_SUCH_OBJECT:
return ENOENT;
- default:
}return EIO;
Is this mapping ldb_error to ernno different as in sysdb_error_to_errno? If no then it would be good to reuse existing fucntion.
LS
On Fri, Aug 26, 2016 at 09:49:27AM +0200, Lukas Slebodnik wrote:
On (26/08/16 08:10), Fabiano Fidêncio wrote:
Jakub,
On Thu, Aug 25, 2016 at 1:56 PM, Jakub Hrozek jhrozek@redhat.com wrote:
On Wed, Aug 17, 2016 at 03:13:57PM +0200, Fabiano Fidêncio wrote:
The attached patch for #3125 [0]is based on Jakub's "secrets" branch on github [1], as there are (at least) a few issues with the current secrets code.
Although I didn't fire a CI build, the local "make intgcheck" passes without issues.
Best Regards,
Fabiano Fidêncio
Sorry about the late reply. The patch works fine as I would expect:
sudo curl -H "Content-Type: application/json" --unix-socket /var/run/secrets.socket -XDELETE http://localhost/secrets/xxx
<html> <head> <title>404 Not Found</title></head> <body> <h1>Not Found</h1> <p>The requested resource was not found.</p> </body>
Nonetheless, I think the patch can be a bit simpler:
From bc39f7ba0ffa75a9ca972266bbc2741cbb0c8c73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= fidencio@redhat.com Date: Wed, 17 Aug 2016 13:12:21 +0200 Subject: [PATCH] SECRETS: Check whether a secret exists before trying to delete it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
Checking whether a secret exists or not before trying to delete it allows SSSD to report the proper error code in case of trying to delete a non-existent secret.
Resolves: https://fedorahosted.org/sssd/ticket/3125
Signed-off-by: Fabiano Fidêncio fidencio@redhat.com
src/responder/secrets/local.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/src/responder/secrets/local.c b/src/responder/secrets/local.c index e91766f..3985b6d 100644 --- a/src/responder/secrets/local.c +++ b/src/responder/secrets/local.c @@ -376,12 +376,26 @@ int local_db_delete(TALLOC_CTX *mem_ctx, struct local_context *lctx, const char *req_path) {
static const char *attrs[] = { "secret", NULL }; struct ldb_dn *dn;
struct ldb_result *res; int ret;
ret = local_db_dn(mem_ctx, lctx->ldb, req_path, &dn); if (ret != EOK) goto done;
ret = ldb_search(lctx->ldb, mem_ctx, &res, dn, LDB_SCOPE_BASE,
attrs, "%s", LOCAL_SIMPLE_FILTER);
if (ret != EOK) {
ret = ENOENT;
goto done;
}
if (res->count == 0) {
ret = ENOENT;
goto done;
}
ret = ldb_delete(lctx->ldb, dn);
I think it's OK to just try to delete the secret and act on the return value. This is what we already have in sysdb_delete_cache_entry(): 105 ret = ldb_delete(ldb, dn); 106 switch (ret) { 107 case LDB_SUCCESS: 108 return EOK; 109 case LDB_ERR_NO_SUCH_OBJECT: 110 if (ignore_not_found) { 111 return EOK; 112 }
So I think we can use something else in secrets to return ENOENT on receiving LDB_ERR_NO_SUCH_OBJECT from ldb_delete().
Suggestion taken. You can find the v2 attached to this email.
Best Regards,
Fabiano Fidêncio
From 3e71f901ab12f5e5c570dcf0796ef47e379e7a53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= fidencio@redhat.com Date: Wed, 17 Aug 2016 13:12:21 +0200 Subject: [PATCH v2] SECRETS: Return ENOENT when_deleting a non-existent secret MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
The approach taken is quite similar to what is done by sysdb_delete_cache_entry().
Resolves: https://fedorahosted.org/sssd/ticket/3125
Signed-off-by: Fabiano Fidêncio fidencio@redhat.com
src/responder/secrets/local.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/responder/secrets/local.c b/src/responder/secrets/local.c index e91766f..c32af8c 100644 --- a/src/responder/secrets/local.c +++ b/src/responder/secrets/local.c @@ -380,15 +380,17 @@ int local_db_delete(TALLOC_CTX *mem_ctx, int ret;
ret = local_db_dn(mem_ctx, lctx->ldb, req_path, &dn);
- if (ret != EOK) goto done;
if (ret != EOK) return ret;
ret = ldb_delete(lctx->ldb, dn);
- if (ret != EOK) {
ret = EIO;
- switch (ret) {
- case LDB_SUCCESS:
return EOK;
- case LDB_ERR_NO_SUCH_OBJECT:
return ENOENT;
- default:
}return EIO;
Is this mapping ldb_error to ernno different as in sysdb_error_to_errno? If no then it would be good to reuse existing fucntion.
As long as the errno codes would not translate into some unexpected HTTP error code, I agree.
On Fri, Aug 26, 2016 at 9:49 AM, Lukas Slebodnik lslebodn@redhat.com wrote:
On (26/08/16 08:10), Fabiano Fidêncio wrote:
Jakub,
On Thu, Aug 25, 2016 at 1:56 PM, Jakub Hrozek jhrozek@redhat.com wrote:
On Wed, Aug 17, 2016 at 03:13:57PM +0200, Fabiano Fidêncio wrote:
The attached patch for #3125 [0]is based on Jakub's "secrets" branch on github [1], as there are (at least) a few issues with the current secrets code.
Although I didn't fire a CI build, the local "make intgcheck" passes without issues.
Best Regards,
Fabiano Fidêncio
Sorry about the late reply. The patch works fine as I would expect:
sudo curl -H "Content-Type: application/json" --unix-socket /var/run/secrets.socket -XDELETE http://localhost/secrets/xxx
<html> <head> <title>404 Not Found</title></head> <body> <h1>Not Found</h1> <p>The requested resource was not found.</p> </body>
Nonetheless, I think the patch can be a bit simpler:
From bc39f7ba0ffa75a9ca972266bbc2741cbb0c8c73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= fidencio@redhat.com Date: Wed, 17 Aug 2016 13:12:21 +0200 Subject: [PATCH] SECRETS: Check whether a secret exists before trying to delete it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
Checking whether a secret exists or not before trying to delete it allows SSSD to report the proper error code in case of trying to delete a non-existent secret.
Resolves: https://fedorahosted.org/sssd/ticket/3125
Signed-off-by: Fabiano Fidêncio fidencio@redhat.com
src/responder/secrets/local.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/src/responder/secrets/local.c b/src/responder/secrets/local.c index e91766f..3985b6d 100644 --- a/src/responder/secrets/local.c +++ b/src/responder/secrets/local.c @@ -376,12 +376,26 @@ int local_db_delete(TALLOC_CTX *mem_ctx, struct local_context *lctx, const char *req_path) {
static const char *attrs[] = { "secret", NULL }; struct ldb_dn *dn;
struct ldb_result *res; int ret;
ret = local_db_dn(mem_ctx, lctx->ldb, req_path, &dn); if (ret != EOK) goto done;
ret = ldb_search(lctx->ldb, mem_ctx, &res, dn, LDB_SCOPE_BASE,
attrs, "%s", LOCAL_SIMPLE_FILTER);
if (ret != EOK) {
ret = ENOENT;
goto done;
}
if (res->count == 0) {
ret = ENOENT;
goto done;
}
ret = ldb_delete(lctx->ldb, dn);
I think it's OK to just try to delete the secret and act on the return value. This is what we already have in sysdb_delete_cache_entry(): 105 ret = ldb_delete(ldb, dn); 106 switch (ret) { 107 case LDB_SUCCESS: 108 return EOK; 109 case LDB_ERR_NO_SUCH_OBJECT: 110 if (ignore_not_found) { 111 return EOK; 112 }
So I think we can use something else in secrets to return ENOENT on receiving LDB_ERR_NO_SUCH_OBJECT from ldb_delete().
Suggestion taken. You can find the v2 attached to this email.
Best Regards,
Fabiano Fidêncio
From 3e71f901ab12f5e5c570dcf0796ef47e379e7a53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= fidencio@redhat.com Date: Wed, 17 Aug 2016 13:12:21 +0200 Subject: [PATCH v2] SECRETS: Return ENOENT when_deleting a non-existent secret MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
The approach taken is quite similar to what is done by sysdb_delete_cache_entry().
Resolves: https://fedorahosted.org/sssd/ticket/3125
Signed-off-by: Fabiano Fidêncio fidencio@redhat.com
src/responder/secrets/local.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/responder/secrets/local.c b/src/responder/secrets/local.c index e91766f..c32af8c 100644 --- a/src/responder/secrets/local.c +++ b/src/responder/secrets/local.c @@ -380,15 +380,17 @@ int local_db_delete(TALLOC_CTX *mem_ctx, int ret;
ret = local_db_dn(mem_ctx, lctx->ldb, req_path, &dn);
- if (ret != EOK) goto done;
if (ret != EOK) return ret;
ret = ldb_delete(lctx->ldb, dn);
- if (ret != EOK) {
ret = EIO;
- switch (ret) {
- case LDB_SUCCESS:
return EOK;
- case LDB_ERR_NO_SUCH_OBJECT:
return ENOENT;
- default:
}return EIO;
Is this mapping ldb_error to ernno different as in sysdb_error_to_errno? If no then it would be good to reuse existing fucntion.
Hmm. Interesting. Didn't even know about this function. Suggestion taken, see v3 attached.
Best Regards, -- Fabiano Fidêncio
On Fri, Aug 26, 2016 at 10:01:56AM +0200, Fabiano Fidêncio wrote:
On Fri, Aug 26, 2016 at 9:49 AM, Lukas Slebodnik lslebodn@redhat.com wrote:
On (26/08/16 08:10), Fabiano Fidêncio wrote:
Jakub,
On Thu, Aug 25, 2016 at 1:56 PM, Jakub Hrozek jhrozek@redhat.com wrote:
On Wed, Aug 17, 2016 at 03:13:57PM +0200, Fabiano Fidêncio wrote:
The attached patch for #3125 [0]is based on Jakub's "secrets" branch on github [1], as there are (at least) a few issues with the current secrets code.
Although I didn't fire a CI build, the local "make intgcheck" passes without issues.
Best Regards,
Fabiano Fidêncio
Sorry about the late reply. The patch works fine as I would expect:
sudo curl -H "Content-Type: application/json" --unix-socket /var/run/secrets.socket -XDELETE http://localhost/secrets/xxx
<html> <head> <title>404 Not Found</title></head> <body> <h1>Not Found</h1> <p>The requested resource was not found.</p> </body>
Nonetheless, I think the patch can be a bit simpler:
From bc39f7ba0ffa75a9ca972266bbc2741cbb0c8c73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= fidencio@redhat.com Date: Wed, 17 Aug 2016 13:12:21 +0200 Subject: [PATCH] SECRETS: Check whether a secret exists before trying to delete it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
Checking whether a secret exists or not before trying to delete it allows SSSD to report the proper error code in case of trying to delete a non-existent secret.
Resolves: https://fedorahosted.org/sssd/ticket/3125
Signed-off-by: Fabiano Fidêncio fidencio@redhat.com
src/responder/secrets/local.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/src/responder/secrets/local.c b/src/responder/secrets/local.c index e91766f..3985b6d 100644 --- a/src/responder/secrets/local.c +++ b/src/responder/secrets/local.c @@ -376,12 +376,26 @@ int local_db_delete(TALLOC_CTX *mem_ctx, struct local_context *lctx, const char *req_path) {
static const char *attrs[] = { "secret", NULL }; struct ldb_dn *dn;
struct ldb_result *res; int ret;
ret = local_db_dn(mem_ctx, lctx->ldb, req_path, &dn); if (ret != EOK) goto done;
ret = ldb_search(lctx->ldb, mem_ctx, &res, dn, LDB_SCOPE_BASE,
attrs, "%s", LOCAL_SIMPLE_FILTER);
if (ret != EOK) {
ret = ENOENT;
goto done;
}
if (res->count == 0) {
ret = ENOENT;
goto done;
}
ret = ldb_delete(lctx->ldb, dn);
I think it's OK to just try to delete the secret and act on the return value. This is what we already have in sysdb_delete_cache_entry(): 105 ret = ldb_delete(ldb, dn); 106 switch (ret) { 107 case LDB_SUCCESS: 108 return EOK; 109 case LDB_ERR_NO_SUCH_OBJECT: 110 if (ignore_not_found) { 111 return EOK; 112 }
So I think we can use something else in secrets to return ENOENT on receiving LDB_ERR_NO_SUCH_OBJECT from ldb_delete().
Suggestion taken. You can find the v2 attached to this email.
Best Regards,
Fabiano Fidêncio
From 3e71f901ab12f5e5c570dcf0796ef47e379e7a53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= fidencio@redhat.com Date: Wed, 17 Aug 2016 13:12:21 +0200 Subject: [PATCH v2] SECRETS: Return ENOENT when_deleting a non-existent secret MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
The approach taken is quite similar to what is done by sysdb_delete_cache_entry().
Resolves: https://fedorahosted.org/sssd/ticket/3125
Signed-off-by: Fabiano Fidêncio fidencio@redhat.com
src/responder/secrets/local.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/responder/secrets/local.c b/src/responder/secrets/local.c index e91766f..c32af8c 100644 --- a/src/responder/secrets/local.c +++ b/src/responder/secrets/local.c @@ -380,15 +380,17 @@ int local_db_delete(TALLOC_CTX *mem_ctx, int ret;
ret = local_db_dn(mem_ctx, lctx->ldb, req_path, &dn);
- if (ret != EOK) goto done;
if (ret != EOK) return ret;
ret = ldb_delete(lctx->ldb, dn);
- if (ret != EOK) {
ret = EIO;
- switch (ret) {
- case LDB_SUCCESS:
return EOK;
- case LDB_ERR_NO_SUCH_OBJECT:
return ENOENT;
- default:
}return EIO;
Is this mapping ldb_error to ernno different as in sysdb_error_to_errno? If no then it would be good to reuse existing fucntion.
Hmm. Interesting. Didn't even know about this function. Suggestion taken, see v3 attached.
Best Regards,
Fabiano Fidêncio
Thanks, ACK (still works, code looks good)
sssd-devel@lists.fedorahosted.org