You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "bzp2010 (via GitHub)" <gi...@apache.org> on 2025/08/14 20:07:52 UTC

[PR] feat: add last modified and digest metadata to standalone API [apisix]

bzp2010 opened a new pull request, #12526:
URL: https://github.com/apache/apisix/pull/12526

   ### Description
   
   1. Add a new metadata response header to the Standalone API. This includes `X-Last-Modified` and `X-Digest`. 
   They are used to report to a cluster of multiple APISIX instances, which instance was last updated, and the configuration digest currently in use.
   The `X-Digest` is passed in by the `PUT` request and recorded in memory, and then output in the response header of a `GET` or `HEAD` request. This value is transparent to APISIX, which does not parse or generate the value in any way; it is completely controlled by the client. There is a simple judgment here; if the digest input for the PUT request matches what is currently stored in memory, the API will return 204, and the configuration update will not actually occur.
   
   2. Add support for the `HEAD` method, which returns only metadata but not configuration.
   
   ### Checklist
   
   - [x] I have explained the need for this PR and the problem it solves
   - [x] I have explained the changes or the new features added to this PR
   - [x] I have added tests corresponding to this change
   - [ ] I have updated the documentation to reflect this change
   - [ ] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first) 
   
   I explicitly point out that this is not backwards compatible. Since this feature is clearly marked as experimental and users have been warned not to rely on it, we can comfortably make the changes we need.
   
   <!--
   
   Note
   
   1. Mark the PR as draft until it's ready to be reviewed.
   2. Always add/update tests for any changes unless you have a good reason.
   3. Always update the documentation to reflect the changes made in the PR.
   4. Make a new commit to resolve conversations instead of `push -f`.
   5. To resolve merge conflicts, merge master instead of rebasing.
   6. Use "request review" to notify the reviewer after making changes.
   7. Only a reviewer can mark a conversation as resolved.
   
   -->
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat: add last modified and digest metadata to standalone API [apisix]

Posted by "bzp2010 (via GitHub)" <gi...@apache.org>.
bzp2010 commented on code in PR #12526:
URL: https://github.com/apache/apisix/pull/12526#discussion_r2281301848


##########
apisix/admin/standalone.lua:
##########
@@ -233,11 +252,17 @@ local function update(ctx)
         end
     end
 
+    -- write metadata
+    apisix_yaml[METADATA_LAST_MODIFIED] = ngx_time()

Review Comment:
   This is intentional, I want to make sure that the record is atomic, succeeding at the same time or failing at the same time. If it partially succeeds, this will lead to accidents.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat: add last modified and digest metadata to standalone API [apisix]

Posted by "bzp2010 (via GitHub)" <gi...@apache.org>.
bzp2010 commented on code in PR #12526:
URL: https://github.com/apache/apisix/pull/12526#discussion_r2281341489


##########
apisix/admin/standalone.lua:
##########
@@ -286,15 +311,37 @@ local function get(ctx)
     if not resp then
         return core.response.exit(500, {error_msg = err})
     end
+
+    core.response.set_header(METADATA_LAST_MODIFIED, config and config[METADATA_LAST_MODIFIED])
+    core.response.set_header(METADATA_DIGEST, config and config[METADATA_DIGEST])
     return core.response.exit(200, resp)
 end
 
 
+local function head(ctx)
+    local config, err = get_config()
+    if not config then
+        if err ~= "not found" then
+            core.log.error("failed to get config from shared dict: ", err)
+            return core.response.exit(500, {
+                error_msg = "failed to get config from shared dict: " .. err
+            })
+        end

Review Comment:
   Only `not found` is skipped by this if, which is expected to exist, so it seems fine.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat: add last modified and digest metadata to standalone API [apisix]

Posted by "bzp2010 (via GitHub)" <gi...@apache.org>.
bzp2010 commented on code in PR #12526:
URL: https://github.com/apache/apisix/pull/12526#discussion_r2281377556


##########
apisix/admin/standalone.lua:
##########
@@ -138,9 +143,16 @@ end
 
 
 local function update(ctx)
-    local content_type = core.request.header(nil, "content-type") or "application/json"
+    -- check digest header existence
+    local digest = core.request.header(nil, METADATA_DIGEST)
+    if not digest then

Review Comment:
   I clearly stated this in the PR description.
   
   1. This is a break change
   2. This is necessary to ensure global consistency in new releases.
   3. I'm not obligated to guarantee backward compatibility of an experimental feature. Upon my request, the feature was documented as experimental in the 3.13 documentation.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat: add last modified and digest metadata to standalone API [apisix]

Posted by "SkyeYoung (via GitHub)" <gi...@apache.org>.
SkyeYoung commented on PR #12526:
URL: https://github.com/apache/apisix/pull/12526#issuecomment-3190291284

   Please fix the errors reported in the CI.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat: add last modified and digest metadata to standalone API [apisix]

Posted by "bzp2010 (via GitHub)" <gi...@apache.org>.
bzp2010 merged PR #12526:
URL: https://github.com/apache/apisix/pull/12526


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat: add last modified and digest metadata to standalone API [apisix]

Posted by "membphis (via GitHub)" <gi...@apache.org>.
membphis commented on code in PR #12526:
URL: https://github.com/apache/apisix/pull/12526#discussion_r2281398372


##########
apisix/admin/standalone.lua:
##########
@@ -138,9 +143,16 @@ end
 
 
 local function update(ctx)
-    local content_type = core.request.header(nil, "content-type") or "application/json"
+    -- check digest header existence
+    local digest = core.request.header(nil, METADATA_DIGEST)
+    if not digest then

Review Comment:
   I reviewed again, agree with you



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat: add last modified and digest metadata to standalone API [apisix]

Posted by "membphis (via GitHub)" <gi...@apache.org>.
membphis commented on code in PR #12526:
URL: https://github.com/apache/apisix/pull/12526#discussion_r2281357449


##########
apisix/admin/standalone.lua:
##########
@@ -138,9 +143,16 @@ end
 
 
 local function update(ctx)
-    local content_type = core.request.header(nil, "content-type") or "application/json"
+    -- check digest header existence
+    local digest = core.request.header(nil, METADATA_DIGEST)
+    if not digest then

Review Comment:
   I don't know why this "digest" is not optional?
   
   It seems to be a break change



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat: add last modified and digest metadata to standalone API [apisix]

Posted by "SkyeYoung (via GitHub)" <gi...@apache.org>.
SkyeYoung commented on code in PR #12526:
URL: https://github.com/apache/apisix/pull/12526#discussion_r2281168189


##########
apisix/admin/standalone.lua:
##########
@@ -286,15 +311,37 @@ local function get(ctx)
     if not resp then
         return core.response.exit(500, {error_msg = err})
     end
+
+    core.response.set_header(METADATA_LAST_MODIFIED, config and config[METADATA_LAST_MODIFIED])
+    core.response.set_header(METADATA_DIGEST, config and config[METADATA_DIGEST])
     return core.response.exit(200, resp)
 end
 
 
+local function head(ctx)
+    local config, err = get_config()
+    if not config then
+        if err ~= "not found" then
+            core.log.error("failed to get config from shared dict: ", err)
+            return core.response.exit(500, {
+                error_msg = "failed to get config from shared dict: " .. err
+            })
+        end

Review Comment:
   add some logs for other types of errors?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat: add last modified and digest metadata to standalone API [apisix]

Posted by "bzp2010 (via GitHub)" <gi...@apache.org>.
bzp2010 commented on code in PR #12526:
URL: https://github.com/apache/apisix/pull/12526#discussion_r2281377556


##########
apisix/admin/standalone.lua:
##########
@@ -138,9 +143,16 @@ end
 
 
 local function update(ctx)
-    local content_type = core.request.header(nil, "content-type") or "application/json"
+    -- check digest header existence
+    local digest = core.request.header(nil, METADATA_DIGEST)
+    if not digest then

Review Comment:
   I clearly stated this in the PR description.
   
   1. This is a break change
   2. This is necessary to ensure global consistency in new releases.
   3. I'm not obligated to guarantee backward compatibility of an experimental feature



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat: add last modified and digest metadata to standalone API [apisix]

Posted by "AlinsRan (via GitHub)" <gi...@apache.org>.
AlinsRan commented on code in PR #12526:
URL: https://github.com/apache/apisix/pull/12526#discussion_r2281124876


##########
apisix/admin/standalone.lua:
##########
@@ -233,11 +252,17 @@ local function update(ctx)
         end
     end
 
+    -- write metadata
+    apisix_yaml[METADATA_LAST_MODIFIED] = ngx_time()

Review Comment:
   You might consider storing `METADATA_LAST_MODIFIED` and `METADATA_DIGEST` in a separate shared memory key rather than mixing them into apisix_yaml, so as not to affect the diff logic and to keep the config structure cleaner.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org